From 13f61f0965f27cfe41a9c72aa24a87bbf6659e9e Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Mon, 7 Sep 2026 16:49:37 +0200 Subject: [PATCH 1/5] feat(intercom): contacts, companies and contact relations Publishes the two collections phase 2 is walked through, and turns the denormalized contact columns of lot 1 into a relation now that their target exists. Contacts read through the one endpoint of the API that sorts, so the measured table gained a sortable flag and the cursor tier a server-side order; the match-all predicate Tickets already sent is what routes a list view asking for an order through the search. Companies read through the third pagination tier, by offset, which is the one place the window a list view asks for maps onto what Intercom takes. In exchange it is looked up rather than searched, and anything past the two published lookups is refused by name. Contact and company custom attributes are typed from GET /data_attributes and carry api_writable for the lot that writes, though every column here is read-only. Resolving a relation condition is now bounded on the cursor tier: the target is read one record past what a group may hold, rather than walking a whole collection to refuse the fan-out afterwards. contact_email and contact_ids give way to contact_id and the contact relation, one readable label plus navigation, which is the rule lot 2.5 set for the ticket labels. No saved view can rest on them: both were already refused server-side. Co-Authored-By: Claude Opus 5 (1M context) --- .../README.md | 6 +- .../client.rb | 73 ++- .../collections/company.rb | 83 ++++ .../collections/company/serializer.rb | 52 ++ .../collections/contact.rb | 198 ++++++++ .../collections/contact/serializer.rb | 115 +++++ .../collections/contact_identity.rb | 41 +- .../collections/conversation.rb | 28 +- .../collections/cursor_collection.rb | 136 ++++-- .../collections/offset_collection.rb | 303 ++++++++++++ .../collections/relations.rb | 31 +- .../collections/ticket.rb | 23 +- .../datasource.rb | 22 +- .../query/search_fields.rb | 26 +- .../query/search_fields.yml | 275 ++++++++++- .../schema/data_attributes_introspector.rb | 119 +++++ .../collections/company_spec.rb | 354 ++++++++++++++ .../collections/contact_spec.rb | 445 ++++++++++++++++++ .../collections/conversation_spec.rb | 25 +- .../collections/ticket_spec.rb | 8 +- .../datasource_spec.rb | 36 +- .../query/search_fields_spec.rb | 38 +- .../data_attributes_introspector_spec.rb | 136 ++++++ .../spec/probe_search_fields_spec.rb | 4 +- .../spec/spec_helper.rb | 19 +- 25 files changed, 2446 insertions(+), 150 deletions(-) create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/offset_collection.rb create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/schema/data_attributes_introspector.rb create mode 100644 packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb create mode 100644 packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb create mode 100644 packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/schema/data_attributes_introspector_spec.rb diff --git a/packages/forest_admin_datasource_intercom/README.md b/packages/forest_admin_datasource_intercom/README.md index 43ce826e8..c44ed9995 100644 --- a/packages/forest_admin_datasource_intercom/README.md +++ b/packages/forest_admin_datasource_intercom/README.md @@ -250,8 +250,10 @@ What follows from that: | Collection | Filterable on | | --- | --- | -| `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` | +| `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`, `contact_id`, 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`, `contact_id`, `created_at`, `updated_at` | +| `IntercomContact` | `id`, `role`, `name`, `email`, `email_domain`, `phone`, `external_id`, `owner_id`, `unsubscribed_from_emails`, `has_hard_bounced`, `marked_email_as_spam`, `language_override`, `browser`, `browser_language`, `os`, `location_country`, `location_region`, `location_city`, and the dates `created_at`, `updated_at`, `signed_up_at`, `last_seen_at`, `last_contacted_at`, `last_replied_at`, `last_email_opened_at`, `last_email_clicked_at` | +| `IntercomCompany` | `id`, `company_id`, `name` — four lookups and no search endpoint, see [Companies](#companies) | **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 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 ecfe093b3..46b9008d3 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 @@ -24,7 +24,9 @@ class Client # rubocop:disable Metrics/ClassLength # callers never have to know how the absence is spelled on the wire. # `total_count` is exact, filter included, which is what makes Forest's # record counter and its "number of" charts one request each. - Page = Struct.new(:records, :next_cursor, :total_count, keyword_init: true) + # `total_pages` is filled by the one endpoint that paginates by offset and + # nil everywhere else: a cursor page has no notion of how many there are. + Page = Struct.new(:records, :next_cursor, :total_count, :total_pages, keyword_init: true) def initialize(configuration) @configuration = configuration @@ -65,14 +67,44 @@ def list_page(path, per_page:, starting_after: nil, params: {}, list_key: 'data' # 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: {}) + # `sort` is honoured by `/contacts/search` alone. The other two search + # endpoints accept one and ignore it without a word -- measured -- so the + # collections that read them never send one, and the parameter is here for + # the one that does. `{ field:, ascending: }`, translated to Intercom's own + # spelling on the way out. + def search_page(path, query:, per_page:, starting_after: nil, list_key: 'data', params: {}, sort: nil) pagination = { 'per_page' => self.class.bounded_per_page(per_page) } pagination['starting_after'] = starting_after unless blank?(starting_after) body = { 'query' => query, 'pagination' => pagination } + body['sort'] = sort_clause(sort) if sort must_succeed(path) { to_page(post(path, body, params: params).body, path, list_key) } end + # One page of an endpoint that paginates by **offset** rather than by + # cursor. `POST /companies/list` is the only one, and it is what lets the + # Companies collection answer page 7 of a list view with one request + # instead of walking six pages to reach it. Intercom counts pages from 1. + def offset_page(path, page:, per_page:, params: {}, list_key: 'data') + query = params.merge('page' => [page.to_i, 1].max, + 'per_page' => self.class.bounded_per_page(per_page)) + + must_succeed(path) { to_page(post(path, {}, params: query).body, path, list_key) } + end + + # An exact lookup, and the shape surprise that comes with it: `GET + # /companies?name=` answers the company itself where `?tag_id=` answers a + # list. A record is read here as a page of one, so a caller writes one route + # rather than testing the envelope. + def lookup_page(path, params:, list_key: 'data') + must_succeed(path) do + body = get(path, params).body + next Page.new(records: [body], next_cursor: nil, total_count: 1) if single_record?(body, list_key) + + to_page(body, path, list_key) + end + end + # One record from its own endpoint. Raises on a 404 like on any other # failure: what a missing record means -- a stale link, a record outside the # token's scope, a deletion -- is the caller's to decide, not the client's. @@ -98,8 +130,11 @@ def fetch_record(path, id, params: {}, boot: false) # parameter in the specification is not a promise that a large workspace # answers in one response, and a truncated reference collection would show # an operator a state list missing its last states. - def fetch_all(path, list_key: 'data', boot: false) - must_succeed(path) { collect_pages(path, list_key: list_key, boot: boot) } + # `params` is what narrows the endpoint rather than what pages it: + # `/data_attributes` answers the attributes of contacts and those of + # companies under `?model=`, and both are read whole. + def fetch_all(path, list_key: 'data', params: {}, boot: false) + must_succeed(path) { collect_pages(path, list_key: list_key, params: params, boot: boot) } end # The page size Intercom accepts, whatever was asked for. @@ -149,13 +184,28 @@ def verify_pinned_version(response) ) end - def collect_pages(path, list_key:, boot:) + # Intercom spells an order `{ "field": "...", "order": "descending" }`, + # and answers `data_invalid` on anything else -- so the clause is written + # here rather than by the caller, whose vocabulary is Forest's. + def sort_clause(sort) + { 'field' => sort[:field].to_s, 'order' => sort[:ascending] == false ? 'descending' : 'ascending' } + end + + # A record rather than a listing: no list under either key, and an id where + # a record carries one. An empty listing is not one of these -- it answers + # `data` as an empty array, which is a page of nothing rather than a record. + def single_record?(body, list_key) + body.is_a?(Hash) && !body[list_key].is_a?(Array) && !body['data'].is_a?(Array) && body.key?('id') + end + + def collect_pages(path, list_key:, params:, boot:) records = [] cursor = nil pages = 0 loop do - body = get(path, cursor.nil? ? nil : { 'starting_after' => cursor }, boot: boot).body + query = cursor.nil? ? params : params.merge('starting_after' => cursor) + body = get(path, query.empty? ? nil : query, boot: boot).body records.concat(extract_entities(body, path, list_key)) pages += 1 cursor = next_cursor(body, path) @@ -199,7 +249,8 @@ def log_collection_cap(path, pages, collected) def to_page(body, operation, list_key) Page.new(records: extract_entities(body, operation, list_key), next_cursor: next_cursor(body, operation), - total_count: extract_count(body)) + total_count: extract_count(body), + total_pages: extract_total_pages(body)) end # Absent on the last page, which is how the walk knows it is done. An older @@ -236,6 +287,14 @@ def extract_count(body) count.is_a?(Numeric) ? count.to_i : nil end + # How many pages the offset tier has to read through, when the endpoint + # counts them. nil on a cursor page, which counts nothing. + def extract_total_pages(body) + pages = body['pages'] if body.is_a?(Hash) + total = pages['total_pages'] if pages.is_a?(Hash) + total.is_a?(Numeric) ? total.to_i : nil + end + def refuse_body_shape(operation, detail) raise APIError.new("Intercom API call failed: #{operation}: unexpected response shape, #{detail}", status: nil) end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb new file mode 100644 index 000000000..cb8563ef5 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb @@ -0,0 +1,83 @@ +module ForestAdminDatasourceIntercom + module Collections + # The accounts the contacts belong to. + # + # The one collection Intercom paginates by offset, and the one place R1 -- + # a window the API cannot express -- does not apply: `POST /companies/list` + # takes a page number, which is what a list view asks for. See + # `OffsetCollection`. + # + # In exchange it is the least filterable collection of the datasource. + # There is no `/companies/search`, and what `GET /companies` answers is four + # exact lookups: by `name`, by `company_id` -- the workspace's own + # identifier, not Intercom's -- by `tag_id` and by `segment_id`. Two of them + # are published as filters here, the two that name a column of this + # collection; a tag and a segment are collections of their own and arrive + # with lot 5, which is where filtering by them belongs. + # + # `GET /companies/scroll` is deliberately rejected rather than used: one + # open scroll per application, expiring after a minute, cannot serve two + # operators looking at a list at the same time. + class Company < OffsetCollection + include Company::Serializer + + # The column each lookup is written on, and the query parameter Intercom + # answers it under. They happen to share a name; keeping the mapping + # explicit is what lets a column be renamed without silently dropping the + # lookup. + LOOKUPS = { 'name' => 'name', 'company_id' => 'company_id' }.freeze + + def initialize(datasource, attributes: []) + @attributes = attributes + super(datasource, 'IntercomCompany') + end + + protected + + def list_path = 'companies/list' + def record_endpoint = 'companies' + def lookups = LOOKUPS + + private + + def define_schema + add_column('id', 'String', is_primary_key: true) + # Intercom's id and the workspace's own are two different things, and an + # ops team knows the second one: it is what their billing system calls + # the account. + add_column('company_id', 'String') + add_column('name', 'String') + define_profile_columns + define_activity_columns + define_attribute_columns + add_one_to_many('contacts', foreign_collection: 'IntercomContact', origin_key: 'company_id') + end + + def define_profile_columns + add_column('plan_name', 'String') + add_column('size', 'Number') + add_column('industry', 'String') + add_column('website', 'String') + add_column('monthly_spend', 'Number') + end + + def define_activity_columns + add_column('user_count', 'Number') + add_column('session_count', 'Number') + add_column('created_at', 'Date') + add_column('updated_at', 'Date') + add_column('last_request_at', 'Date') + # When the account was created in the customer's own system, which is + # not when Intercom heard about it. + add_column('remote_created_at', 'Date') + end + + # Typed from `GET /data_attributes?model=company`, and unfilterable for + # the same reason as everything else here: this collection is looked up, + # not searched. + def define_attribute_columns + @attributes.each { |attribute| add_column(attribute.column_name, attribute.column_type) } + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb new file mode 100644 index 000000000..4b9eb7fc3 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb @@ -0,0 +1,52 @@ +module ForestAdminDatasourceIntercom + module Collections + class Company < OffsetCollection + # One Intercom company flattened into the row the schema declares. + module Serializer + protected + + def serialize(company) + attrs = company.is_a?(Hash) ? company : {} + plan = attrs['plan'].is_a?(Hash) ? attrs['plan'] : {} + + identity(attrs).merge( + 'plan_name' => plan['name'], + 'user_count' => attrs['user_count'], + 'session_count' => attrs['session_count'] + ).merge(dates_of(attrs)).merge(attribute_columns_for(attrs)) + end + + private + + def identity(attrs) + { + 'id' => stringify_id(attrs['id']), + 'company_id' => stringify_id(attrs['company_id']), + 'name' => attrs['name'], + 'size' => attrs['size'], + 'industry' => attrs['industry'], + 'website' => attrs['website'], + 'monthly_spend' => attrs['monthly_spend'] + } + end + + def dates_of(attrs) + { + 'created_at' => stamp(attrs['created_at']), + 'updated_at' => stamp(attrs['updated_at']), + 'last_request_at' => stamp(attrs['last_request_at']), + 'remote_created_at' => stamp(attrs['remote_created_at']) + } + end + + # Nil rather than absent for an attribute the company does not carry: + # the column exists on every row. + def attribute_columns_for(attrs) + values = attrs['custom_attributes'].is_a?(Hash) ? attrs['custom_attributes'] : {} + + @attributes.to_h { |attribute| [attribute.column_name, values[attribute.name]] } + end + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb new file mode 100644 index 000000000..8db270d4b --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb @@ -0,0 +1,198 @@ +module ForestAdminDatasourceIntercom + module Collections + # The people who write to the workspace: users and leads alike. + # + # Cursor-paginated like conversations and tickets, with one thing no other + # collection of this datasource has -- **Intercom sorts it**. + # `POST /contacts/search` is the only endpoint of the whole API that takes a + # `sort` and applies it, so this is the only collection whose columns are + # published sortable, and the measured table is what says which ones. + # + # Two routes of its own, on top of the three the tier already has: + # + # * a set of ids is read in one request -- `id IN [...]`, which this + # endpoint answers and no other does -- rather than one request per id; + # * `company_id equals X` reads `GET /companies/{id}/contacts`, which is + # what serves the contacts of an account. `/contacts/search` filters no + # company field, so without this route the one relation an ops team walks + # the most would be a refusal. + # + # A contact merged into another **disappears** from the search and from the + # listing: a row whose contact was merged reads as gone rather than as an + # error, which is what a merge means -- the record still exists, under the + # id it was merged into. + # Long by line count only: most of it declares the columns, one call each. + class Contact < CursorCollection # rubocop:disable Metrics/ClassLength + include Contact::Serializer + + # `/contacts/search` demands a query, so a read with no condition of its + # own -- a list view asking for an order -- sends the least noisy + # predicate that matches everything. Every contact has a creation date, + # and a bound at the epoch keeps whatever the day-granular truncation does + # to it harmless. The same predicate `Ticket` sends, for the same reason. + MATCH_EVERY_CONTACT = { 'field' => 'created_at', 'operator' => '>', 'value' => '0' }.freeze + + # How many ids one bulk read carries, and how many a single `id in [...]` + # may name. Both are far above what a page asks for; they keep a scope or + # a customizer naming thousands of ids from turning one list view into a + # rate limit. + IDS_PER_READ = 100 + MAX_IDS_READ = 300 + + def initialize(datasource, attributes: []) + @attributes = attributes + super(datasource, 'IntercomContact') + # Answered on the e-mail address, which is what an ops team types when + # they are looking for someone. Per word, not as a substring -- see the + # README. + enable_search + end + + protected + + def list_endpoint = 'contacts' + def searchable = 'contacts' + def search_column = 'email' + def match_all_query = MATCH_EVERY_CONTACT + + private + + def define_schema + add_column('id', 'String', is_primary_key: true) + define_identity_columns + define_date_columns + define_reachability_columns + define_device_columns + define_attribute_columns + define_relations + end + + def define_identity_columns + add_column('role', 'String') + add_column('name', 'String') + add_column('email', 'String') + add_column('email_domain', 'String') + add_column('phone', 'String') + add_column('external_id', 'String') + add_column('avatar', 'String') + add_column('owner_id', 'String') + add_column('session_count', 'Number') + # The first of the accounts the contact belongs to, and how many there + # are: the same reading a conversation gives its contacts, and the + # foreign key the `company` relation is built on. + add_column('company_id', 'String') + add_column('company_count', 'Number') + end + + def define_date_columns + add_column('created_at', 'Date') + add_column('updated_at', 'Date') + add_column('signed_up_at', 'Date') + add_column('last_seen_at', 'Date') + add_column('last_contacted_at', 'Date') + add_column('last_replied_at', 'Date') + add_column('last_email_opened_at', 'Date') + add_column('last_email_clicked_at', 'Date') + end + + def define_reachability_columns + add_column('unsubscribed_from_emails', 'Boolean') + add_column('has_hard_bounced', 'Boolean') + add_column('marked_email_as_spam', 'Boolean') + end + + def define_device_columns + add_column('language_override', 'String') + add_column('browser', 'String') + add_column('browser_language', 'String') + add_column('os', 'String') + add_column('location_country', 'String') + add_column('location_region', 'String') + add_column('location_city', 'String') + end + + # Typed from `GET /data_attributes?model=contact` rather than guessed from + # a payload, and published unfilterable: which operators Intercom answers + # on `custom_attributes.{name}` has not been measured, and this package + # offers no filter it has not seen work. `api_writable` travels on the + # introspected attribute for lot 4b, not on the column -- everything here + # is read-only. + def define_attribute_columns + @attributes.each { |attribute| add_column(attribute.column_name, attribute.column_type) } + end + + # The owner is a teammate, read whole in one request, and + # `/contacts/search` filters on the key -- so that relation is readable, + # navigable and filterable alike. + # + # The company is none of those last two: the endpoint filters no company + # field, so the traversal is refused by name (see `check_relation_filterable!` + # on the tier). The two lists are the 360 degrees this lot exists for. + def define_relations + add_many_to_one('owner', foreign_collection: 'IntercomAdmin', foreign_key: 'owner_id') + add_many_to_one('company', foreign_collection: 'IntercomCompany', foreign_key: 'company_id') + add_one_to_many('conversations', foreign_collection: 'IntercomConversation', origin_key: 'contact_id') + add_one_to_many('tickets', foreign_collection: 'IntercomTicket', origin_key: 'contact_id') + end + + # The contacts of an account, which `/contacts/search` cannot answer and + # `GET /companies/{id}/contacts` can. Anything else goes the usual way. + def fetch_records(caller, filter, sort = nil) + company = company_lookup(filter) + return super unless company + + warn_ignored_sort(Array(filter&.sort)) if sort + offset, limit = translate_page(filter&.page) + + walker.walk(offset: offset, limit: limit) do |per_page, cursor| + read_company_page(company, per_page: per_page, cursor: cursor) + end + end + + def count_records(caller, filter) + company = company_lookup(filter) + return super unless company + + exact_count(read_company_page(company, per_page: 1, cursor: nil)) + end + + # A bare equality and nothing else: an `and` also carrying a scope names a + # narrower set than the account does, and answering it with the account + # alone would serve contacts the scope excludes. + def company_lookup(filter) + tree = filter&.condition_tree + return nil unless tree.is_a?(Leaf) && tree.field.to_s == 'company_id' + return nil unless tree.operator == Operators::EQUAL && blank_search?(filter) + + tree.value&.to_s + end + + def read_company_page(company, per_page:, cursor:) + client.list_page("companies/#{Faraday::Utils.escape(company)}/contacts", + per_page: [per_page, max_page_size].min, starting_after: cursor) + end + + # One request per hundred ids instead of one per id: this endpoint answers + # `id IN [...]`, which is what lot 1 already reads the contacts of a page + # through. A contact that was merged away is simply absent from the + # answer -- the row reads as gone, not as a failure. + def records_by_ids(ids) + wanted = ids.first(MAX_IDS_READ) + warn_truncated_ids(ids.size) if ids.size > wanted.size + + wanted.each_slice(IDS_PER_READ).flat_map do |chunk| + client.search_page(search_endpoint.path, per_page: chunk.size, + query: { 'field' => 'id', 'operator' => 'IN', + 'value' => chunk }).records + end + end + + def warn_truncated_ids(asked) + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] #{name} was asked for #{asked} records by id and read the first " \ + "#{MAX_IDS_READ}: Intercom reads them #{IDS_PER_READ} at a time. The result is truncated." + ) + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb new file mode 100644 index 000000000..49283736b --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb @@ -0,0 +1,115 @@ +module ForestAdminDatasourceIntercom + module Collections + class Contact < CursorCollection + # One Intercom contact flattened into the row the schema declares. + # Nothing here reads a sub-resource: every value comes from the payload + # the search already returned. + module Serializer + protected + + def serialize(contact) + attrs = contact.is_a?(Hash) ? contact : {} + + native(attrs) + .merge(account_of(attrs['companies'])) + .merge(location_of(attrs['location'])) + .merge(attribute_columns_for(attrs)) + end + + private + + def native(attrs) + identity(attrs).merge(dates_of(attrs)).merge(flags(attrs)).merge(device_of(attrs)) + end + + def identity(attrs) + { + 'id' => stringify_id(attrs['id']), + 'role' => attrs['role'], + 'name' => attrs['name'], + 'email' => attrs['email'], + # Derived rather than read, and filterable all the same: the search + # endpoint carries a field of its own for it, which is what turns + # "everyone at this customer" into a filter instead of a wildcard. + 'email_domain' => domain_of(attrs['email']), + 'phone' => attrs['phone'], + 'external_id' => attrs['external_id'], + 'avatar' => attrs['avatar'], + 'owner_id' => stringify_id(attrs['owner_id']), + 'session_count' => attrs['session_count'] + } + end + + def dates_of(attrs) + { + 'created_at' => stamp(attrs['created_at']), + 'updated_at' => stamp(attrs['updated_at']), + 'signed_up_at' => stamp(attrs['signed_up_at']), + 'last_seen_at' => stamp(attrs['last_seen_at']), + 'last_contacted_at' => stamp(attrs['last_contacted_at']), + 'last_replied_at' => stamp(attrs['last_replied_at']), + 'last_email_opened_at' => stamp(attrs['last_email_opened_at']), + 'last_email_clicked_at' => stamp(attrs['last_email_clicked_at']) + } + end + + def flags(attrs) + { + 'unsubscribed_from_emails' => attrs['unsubscribed_from_emails'], + 'has_hard_bounced' => attrs['has_hard_bounced'], + 'marked_email_as_spam' => attrs['marked_email_as_spam'] + } + end + + def device_of(attrs) + { + 'language_override' => attrs['language_override'], + 'browser' => attrs['browser'], + 'browser_language' => attrs['browser_language'], + 'os' => attrs['os'] + } + end + + # A contact belongs to several accounts, and the row names the first of + # them and counts them -- the same reading a conversation gives its + # contacts. The whole list is a hop away, on the `company` relation. + def account_of(companies) + list = nested_list(companies, 'data') + first = list.first.is_a?(Hash) ? list.first : {} + + { 'company_id' => stringify_id(first['id']), 'company_count' => account_count(companies, list) } + end + + # Intercom caps the accounts it nests on a contact and says how many + # there really are, so the count is read rather than measured on the + # list -- a contact belonging to twelve accounts must not read as + # belonging to the ten the payload had room for. + def account_count(companies, list) + declared = companies['total_count'] if companies.is_a?(Hash) + + declared.is_a?(Numeric) ? declared : list.size + end + + def location_of(location) + attrs = location.is_a?(Hash) ? location : {} + + { 'location_country' => attrs['country'], 'location_region' => attrs['region'], + 'location_city' => attrs['city'] } + end + + # Nil rather than absent for an attribute the contact does not carry: + # the column exists on every row, and an absent key would read as a + # record missing it. + def attribute_columns_for(attrs) + values = attrs['custom_attributes'].is_a?(Hash) ? attrs['custom_attributes'] : {} + + @attributes.to_h { |attribute| [attribute.column_name, values[attribute.name]] } + end + + def domain_of(email) + email.to_s[/@(.+)\z/, 1] + end + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact_identity.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact_identity.rb index 2bc4fab54..c190f459d 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact_identity.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact_identity.rb @@ -1,17 +1,22 @@ module ForestAdminDatasourceIntercom module Collections - # The contact of a conversation or of a ticket, denormalized onto the row. + # The contact of a conversation or of a ticket, on the row itself. # # Intercom nests only the ids -- `{"type": "contact.list", "contacts": - # [{"id": "..."}]}` -- so a name and an e-mail cost a read. That read is done - # once per page, for every row at once, and never per row: a page of 25 rows - # is one request, not 25. + # [{"id": "..."}]}` -- so a name costs a read. That read is done once per + # page, for every row at once, and never per row: a page of 25 rows is one + # request, not 25. # - # It stays a pair of columns rather than a relation because the Contacts - # collection arrives in lot 4, and a relation whose target collection is - # missing is a schema the agent refuses to boot on. + # **Three columns, where lot 1 published four.** Now that the Contacts + # collection exists, the identity is a relation, and the rule lot 2.5 set + # for the ticket labels applies here too: one readable label on the row plus + # the relation to navigate, rather than two ways to read one fact. + # `contact_email` is gone -- it is one hop away, on `contact:email` -- and + # `contact_ids` gave way to `contact_id`, which is a foreign key rather than + # a Json blob no filter could reach. The list of every contact of a group + # conversation is the `contacts` relation. module ContactIdentity - COLUMNS = %w[contact_name contact_email].freeze + COLUMNS = %w[contact_name].freeze # How many ids one `id in [...]` read carries. A page holds fewer than this # in practice; the chunk keeps the request bounded if it ever does not. @@ -20,22 +25,23 @@ module ContactIdentity private def define_contact_columns - add_column('contact_ids', 'Json') + add_column('contact_id', 'String') add_column('contact_count', 'Number') add_column('contact_name', 'String') - add_column('contact_email', 'String') end # A group conversation, or a ticket opened for several people, has more # than one contact: the row names the first and counts them, rather than - # presenting one of several as the one. + # presenting one of several as the one. The `contact` relation resolves + # that same first contact, so the column and the relation cannot disagree; + # the others are reached through the contact's own conversations. def contact_columns_for(attrs) ids = nested_list(attrs['contacts'], 'contacts').filter_map { |contact| stringify_id(contact['id']) } - { 'contact_ids' => ids, 'contact_count' => ids.size, + { 'contact_id' => ids.first, 'contact_count' => ids.size, # Filled by the bulk read below, and left nil when the projection did - # not ask for them. - 'contact_name' => nil, 'contact_email' => nil } + # not ask for it. + 'contact_name' => nil } end def first_contact_id(record) @@ -50,12 +56,11 @@ def embed_contact_identity(records, rows, projection) records.each_with_index do |record, index| identity = identities[first_contact_id(record)] || {} rows[index]['contact_name'] = identity['name'] if rows[index].key?('contact_name') - rows[index]['contact_email'] = identity['email'] if rows[index].key?('contact_email') end end - # A failure costs the two columns and nothing else: an identity that could - # not be read is not a page that could not be served. + # A failure costs the column and nothing else: an identity that could not + # be read is not a page that could not be served. def contact_identities(records) ids = records.filter_map { |record| first_contact_id(record) }.uniq return {} if ids.empty? @@ -69,7 +74,7 @@ def contact_identities(records) rescue APIError => e ForestAdminDatasourceIntercom.logger.warn( "[forest_admin_datasource_intercom] #{name} could not read the contacts of this page (HTTP " \ - "#{e.status || "-"}); the name and e-mail columns are left empty for it." + "#{e.status || "-"}); the name column is left empty for it." ) {} end 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 380caeec9..43cfedb48 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 @@ -10,7 +10,7 @@ module Collections # customers, and rendering third-party HTML inside Forest is neither safe nor # useful (R10). # Long by line count only: most of it declares the columns, one call each. - class Conversation < CursorCollection # rubocop:disable Metrics/ClassLength + class Conversation < CursorCollection include ContactIdentity include Conversation::Serializer include Conversation::Timeline @@ -85,12 +85,21 @@ def define_schema # filtered through alike. # # No relation towards the company: a conversation carries its account as a - # whole object, so the name is already on the row, and the Companies - # collection arrives with lot 4. + # whole object, so the name is already on the row, and Intercom filters no + # company field on this endpoint -- the relation would be navigable and + # not filterable, where the column is already readable. + # + # The contact is a relation as of lot 4, and a filterable one: the + # endpoint matches a conversation against one of its contact ids + # (measured), so `contact:email` resolves against `/contacts/search` and + # is rewritten onto the key. A group conversation names its first contact + # here, like the column does; every contact of it is a hop away, through + # that contact's own conversations. def define_relations add_many_to_one('admin_assignee', foreign_collection: 'IntercomAdmin', foreign_key: 'admin_assignee_id') add_many_to_one('team_assignee', foreign_collection: 'IntercomTeam', foreign_key: 'team_assignee_id') add_many_to_one('closed_by', foreign_collection: 'IntercomAdmin', foreign_key: 'closed_by_id') + add_many_to_one('contact', foreign_collection: 'IntercomContact', foreign_key: 'contact_id') end # Who the conversation sits with, and which account it belongs to. The @@ -103,19 +112,6 @@ def define_assignment_columns add_column('company_name', 'String') end - # The contact identity is denormalized onto the row rather than declared as - # a relation: the Contacts collection arrives in lot 4, and a relation whose - # target collection is missing is a schema the agent refuses to boot on. - # - # A group conversation has several contacts; the row carries the first and - # says how many there are, rather than pretending there is one. - def define_contact_columns - add_column('contact_ids', 'Json') - add_column('contact_count', 'Number') - add_column('contact_name', 'String') - add_column('contact_email', 'String') - end - # The message that opened the conversation lives in `source`, not in the # parts. A timeline built from the parts alone loses it, which is the one # message nobody opens a conversation without wanting to read. 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 62545008f..cb7ec67d9 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 @@ -40,9 +40,7 @@ def initialize(datasource, name) end def list(caller, filter, projection) - warn_ignored_sort(filter&.sort) - - records = fetch_records(caller, filter) + records = fetch_records(caller, filter, server_sort(filter)) # Serialized whole and projected afterwards rather than the other way # round: a projection reaching through a relation names no foreign key, # and the key is where the relation is read from. @@ -101,15 +99,20 @@ 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:, query: nil) + def read_page(per_page:, cursor:, query: nil, sort: nil) size = [per_page, max_page_size].min + # The listing endpoint neither filters nor sorts, and a collection whose + # records are only reachable through the search has no listing at all. + # Either way what routes the read to the search is a query, so the one + # that matches everything stands in for the condition there is none of. + query ||= match_all_query if sort || searchable_only? 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) + params: read_params, list_key: list_key, sort: sort) end end @@ -122,17 +125,43 @@ def read_page(per_page:, cursor:, query: nil) # 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. + # A column is sortable only where the measured table says the endpoint + # sorts, which is `/contacts/search` and nowhere else: the other two + # accept a `sort` and ignore it without a word, so a sortable column there + # would promise an order that never happens. def add_column(name, type, is_primary_key: false) add_field(name, ColumnSchema.new(column_type: type, filter_operators: column_operators(name, is_primary_key), is_primary_key: is_primary_key, is_read_only: true, - is_sortable: false, + is_sortable: sortable_column?(name), is_groupable: false)) end + def sortable_column?(name) + search_endpoint.field(name)&.sortable? == true + end + + # The Intercom query standing in for the condition a read has none of. + # Two collections need one and for different reasons: Intercom exposes no + # `GET /tickets` at all, and an order is applied by the search endpoint + # alone. Nil where the listing endpoint answers both, which is where a + # sort is reported as ignored rather than dropped. + def match_all_query = nil + + # Whether every read of this collection goes through the search, listing + # or not. + def searchable_only? = false + + # Bounded, unlike the tier read whole: one more than a group may hold is + # all it takes to know the fan-out will not fit, and reading further would + # walk a whole collection to refuse it afterwards. + def match_page + ForestAdminDatasourceToolkit::Components::Query::Page.new( + offset: 0, limit: Query::ConditionTreeTranslator::MAX_GROUP_SIZE + 1 + ) + end + def walker @walker ||= Pagination::CursorWalker.new end @@ -147,7 +176,7 @@ def column_operators(name, is_primary_key) field ? Query::OperatorTable.forest_operators(field) : [] end - def fetch_records(caller, filter) + def fetch_records(caller, filter, sort = nil) 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 @@ -162,7 +191,7 @@ def fetch_records(caller, filter) # than sent as a filter that would come back with everything. return [] if query == NOTHING - listed_records(filter, query) + listed_records(filter, query, sort) end # The Intercom query a filter comes down to, or nil for a list view, which @@ -221,9 +250,14 @@ def refuse_unfilterable_key!(leaf, key) "navigated; filter on one of: #{search_endpoint.filterable_columns.join(", ")}." end - def refuse_fan_out!(leaf, key, ids) + def refuse_fan_out!(leaf, key, _ids) + # "more than", never a count: the target is read one record past what a + # group may hold, so what is known is that it does not fit -- printing + # 16 where a workspace holds three thousand would read as a number the + # operator could go and narrow by one. raise UnsupportedOperatorError, - "#{name} cannot filter #{leaf.field.inspect}: it names #{ids.size} records, " \ + "#{name} cannot filter #{leaf.field.inspect}: it names more than " \ + "#{Query::ConditionTreeTranslator::MAX_GROUP_SIZE} records, " \ "#{search_endpoint.path} answers #{key.inspect} one value at a time, and Intercom takes " \ "#{Query::ConditionTreeTranslator::MAX_GROUP_SIZE} conditions per group. Narrow the condition " \ "on the relation, or filter on #{key.inspect} itself." @@ -288,11 +322,11 @@ def records_by_ids(ids) end end - def listed_records(filter, query) + def listed_records(filter, query, sort = nil) offset, limit = translate_page(filter&.page) walker.walk(offset: offset, limit: limit) do |per_page, cursor| - read_page(per_page: per_page, cursor: cursor, query: query) + read_page(per_page: per_page, cursor: cursor, query: query, sort: sort) end end @@ -315,7 +349,13 @@ def count_records(caller, filter) query = translate(caller, filter) return 0 if query == NOTHING - page = read_page(per_page: 1, cursor: nil, query: query) + exact_count(read_page(per_page: 1, cursor: nil, query: query)) + end + + # The count Intercom answered, or nothing at all. Counting the pages a + # walk collected would answer a fraction of the collection as if it were + # the whole of it, which is the one thing this tier does not do. + def exact_count(page) return page.total_count if page.total_count raise UnsupportedOperatorError, @@ -339,33 +379,65 @@ def refuse_search! '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 - # word -- measured -- so an order the operator asked for and did not get - # has to be reported here or nowhere. The ascending primary-key sort the - # agent injects when a request names none is not one of those. - def warn_ignored_sort(sort) - clauses = Array(sort) - return if clauses.empty? || default_pk_sort?(clauses) + # The order Intercom will really apply, written the way the client sends + # it -- or nil, and the order the operator asked for is then reported + # rather than dropped in silence. + # + # The ascending primary-key sort the agent injects when a request names + # none is neither honoured nor reported: it is not an order anybody asked + # for, and `/contacts/search` does not sort on an id anyway. + def server_sort(filter) + clauses = Array(filter&.sort) + return nil if clauses.empty? || default_pk_sort?(clauses) + + honoured = honourable_sort(clauses) + warn_ignored_sort(clauses) if honoured.nil? + honoured + end + # One clause, on a column the measured table says the endpoint sorts. + # Intercom takes a single `{ field, order }` and nothing composite, so a + # second clause is not half-honoured: honouring the first alone would + # order the page by something the operator did not ask for. + def honourable_sort(clauses) + return nil unless clauses.size == 1 + + clause = clauses.first + field = search_endpoint.field(sort_field(clause).to_s) + return nil unless field&.sortable? + + { field: field.field, ascending: ascending?(clause) } + end + + # Intercom accepts a `sort` on the other two endpoints and ignores it + # without a word -- measured -- so an order asked for and not applied has + # to be reported here or nowhere. + def warn_ignored_sort(clauses) ForestAdminDatasourceIntercom.logger.warn( "[forest_admin_datasource_intercom] #{name} was asked to sort on " \ - "#{clauses.map { |clause| clause[:field] || clause["field"] }.join(", ")}, and Intercom ignores a sort on " \ - 'this endpoint without reporting it. The rows come back in the order the API imposes.' + "#{clauses.map { |clause| sort_field(clause) }.join(", ")}, which Intercom does not sort this " \ + 'collection on -- and it ignores a sort it refuses without reporting it. The rows come back in the ' \ + 'order the API imposes.' ) end + def sort_field(clause) = clause[:field] || clause['field'] + + # `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. + def ascending?(clause) + clause.key?(:ascending) ? clause[:ascending] : clause['ascending'] + end + def default_pk_sort?(clauses) 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 + return false unless sort_field(clause).to_s == primary_key + + ascending?(clause) != false end def warn_truncated_ids(asked) diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/offset_collection.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/offset_collection.rb new file mode 100644 index 000000000..3c11ddd7e --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/offset_collection.rb @@ -0,0 +1,303 @@ +module ForestAdminDatasourceIntercom + module Collections + # The third pagination tier, and the only one that maps onto what Forest + # asks for without translating anything: Intercom paginates + # `POST /companies/list` by **offset**, so page 7 of a list view is one + # request rather than six pages walked to reach it. No cursor walker, no + # cap, and no truncation warning. + # + # What it pays for that is filtering. There is no search endpoint for + # companies at all -- `GET /companies/scroll` exists and is deliberately + # rejected, one open scroll per app expiring in a minute cannot serve + # concurrent list views -- so what a filter may say is four exact lookups + # and nothing else. Everything past them is **refused by name**, the rule + # the cursor tier already set: a page served in answer to a filter it + # ignored is the one failure this datasource is built to avoid. + # + # In memory it does nothing: no filter, no sort, no group. What is in hand + # is a page of something larger, exactly like the cursor tier, and the same + # reasoning applies. + # Long by line count only: half of it is the refusals, and a refusal that + # does not say what to do instead is one an operator cannot act on. + class OffsetCollection < BaseCollection # rubocop:disable Metrics/ClassLength + Aggregation = ForestAdminDatasourceToolkit::Components::Query::Aggregation + + # How many records an `id in [...]` read may fetch. One request per id -- + # Intercom has no "read these records" endpoint here either -- so the + # fan-out is bounded rather than turned into a rate limit halfway through + # a page. + MAX_ID_READS = 25 + + # What one page holds when the read names no window: a relation resolving + # its target, a segment, a customizer. A list view always names one. + UNBOUNDED_PAGE_SIZE = Client::MAX_PER_PAGE + + # And how many such pages are read before the answer is cut short. The + # figure only ever applies to a read with no window of its own. + MAX_COLLECTED_PAGES = 10 + + def initialize(datasource, name) + super + enable_count + end + + def list(caller, filter, projection) + warn_ignored_sort(filter&.sort) + + records = fetch_records(filter) + serialized = records.map { |record| serialize(record) } + rows = serialized.map { |record| project(record, projection) } + + embed_relations(caller, serialized, rows, projection) + rows + end + + # Count only, and never a group: `total_count` is exact on every listing, + # while grouping the page in hand would answer a fraction as if it were + # the whole. + def aggregate(_caller, filter, aggregation, _limit = nil) + refuse_unsupported_aggregation!(aggregation) + + [{ 'group' => {}, 'value' => count_records(filter) }] + end + + protected + + # The endpoint that lists the collection by offset, the one that reads a + # record, and the lookups Intercom answers on the listing path. + def list_path = raise(NotImplementedError, "#{self.class} did not implement list_path") + def record_endpoint = raise(NotImplementedError, "#{self.class} did not implement record_endpoint") + def lookup_path = record_endpoint + def lookups = {} + + def serialize(_entity) = raise(NotImplementedError, "#{self.class} did not implement serialize") + + # A column advertises a filter only where Intercom looks the collection up + # by it, so a column cannot offer a filter this tier would then refuse. + # The primary key is the exception and it is not a filter: `id equals X` + # and `id in [...]` are answered by the record endpoint. + # + # Nothing is sortable: the listing takes no order and ordering a page in + # hand would order a fraction of the collection. + def add_column(name, type, is_primary_key: false) + add_field(name, ColumnSchema.new(column_type: type, + filter_operators: column_operators(name, is_primary_key), + is_primary_key: is_primary_key, + is_read_only: true, + is_sortable: false, + is_groupable: false)) + end + + private + + def column_operators(name, is_primary_key) + return [Operators::EQUAL, Operators::IN] if is_primary_key + + lookups.key?(name) ? [Operators::EQUAL] : [] + end + + def fetch_records(filter) + ids = id_lookup(filter) + return records_by_ids(page_window(ids, filter)) if ids + + lookup = lookup_condition(filter) + return page_window(looked_up_records(lookup), filter) if lookup + + refuse_condition!(filter.condition_tree) unless filter&.condition_tree.nil? + + listed_records(filter) + end + + def count_records(filter) + ids = id_lookup(filter) + return records_by_ids(ids).size if ids + + lookup = lookup_condition(filter) + return looked_up_records(lookup).size if lookup + + refuse_condition!(filter.condition_tree) unless filter&.condition_tree.nil? + + exact_count(read_offset_page(page: 1, per_page: 1)) + end + + # The window a list view asked for, read as the page Intercom counts from + # 1. An offset that does not fall on a page boundary is served by reading + # the page it lands in and the ones after it until the window is filled -- + # exactly, rather than by rounding the offset to something the API likes. + def listed_records(filter) + offset, limit = window(filter&.page) + per_page = Client.bounded_per_page(limit || UNBOUNDED_PAGE_SIZE) + skip = offset % per_page + + collected = collect_pages(first_page: (offset / per_page) + 1, per_page: per_page, + wanted: limit && (skip + limit)) + + limit ? (collected[skip, limit] || []) : collected.drop(skip) + end + + def collect_pages(first_page:, per_page:, wanted:) + records = [] + page = first_page + read = 0 + + loop do + answer = read_offset_page(page: page, per_page: per_page) + records.concat(answer.records) + read += 1 + break if last_page?(answer, page) || (wanted && records.size >= wanted) + break if cap_reached?(read, records.size) + + page += 1 + end + + records + end + + def read_offset_page(page:, per_page:) + client.offset_page(list_path, page: page, per_page: per_page) + end + + def last_page?(answer, page) + answer.records.empty? || (answer.total_pages && page >= answer.total_pages) + end + + def cap_reached?(read, collected) + return false if read < MAX_COLLECTED_PAGES + + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] Stopped reading #{name} after #{read} page(s) / #{collected} " \ + 'record(s); the rest is left out. This read named no window of its own, and a list view always does.' + ) + true + end + + # A filter with no page asks for every record it matched; nil is how the + # window says so. + def window(page) + return [0, nil] if page.nil? + + limit = page.limit.to_i + [page.offset.to_i.clamp(0, nil), limit.positive? ? limit : nil] + end + + # A record detail is `id equals X`, and a bulk read of related records is + # `id in [...]`. Only a bare leaf on the primary key takes this route: an + # `and` also carrying a scope names a narrower set than the ids do. + def id_lookup(filter) + tree = filter&.condition_tree + return nil unless tree.is_a?(Leaf) && tree.field.to_s == primary_key + + case tree.operator + when Operators::EQUAL then [tree.value].compact.map(&:to_s) + when Operators::IN then Array(tree.value).compact.map(&:to_s) + end + end + + def lookup_condition(filter) + tree = filter&.condition_tree + return nil unless tree.is_a?(Leaf) && tree.operator == Operators::EQUAL + + parameter = lookups[tree.field.to_s] + parameter && { parameter => tree.value.to_s } + end + + def primary_key + @primary_key ||= fields.find do |_name, field| + field.respond_to?(:is_primary_key) && field.is_primary_key + end&.first + end + + # A record the operator can no longer reach -- deleted, or outside the + # token's scope -- reads as "no record" rather than as a failed page. + def records_by_ids(ids) + wanted = ids.first(MAX_ID_READS) + warn_truncated_ids(ids.size) if ids.size > wanted.size + + wanted.filter_map do |id| + client.fetch_record(record_endpoint, id) + rescue APIError => e + raise unless e.status == 404 + + nil + end + end + + # An exact lookup answers few records -- one, for the keys this publishes + # -- so it is read as a single page. More than that page holds is reported + # rather than dropped in silence. + def looked_up_records(params) + answer = client.lookup_page(lookup_path, params: params) + warn_truncated_lookup(params) if answer.next_cursor + + answer.records + end + + def exact_count(page) + return page.total_count if page.total_count + + raise UnsupportedOperatorError, + "#{name} cannot be counted: Intercom answered this listing without a total_count, and counting the " \ + 'pages the agent read would answer a fraction of the collection as if it were the whole of it.' + end + + def refuse_condition!(tree) + offender = nil + tree.some_leaf { |leaf| offender = leaf } + + raise UnsupportedOperatorError, + "#{name} cannot filter #{(offender&.field).inspect}: Intercom exposes no search endpoint for this " \ + "collection and looks a record up by #{lookups.keys.join(", ")} alone -- one exact value at a time, " \ + 'with no combination and no other operator. Filter on one of those, or reach the record from the ' \ + 'collection next door.' + end + + def refuse_unsupported_aggregation!(aggregation) + return if aggregation.is_a?(Aggregation) && aggregation.operation.to_s.casecmp('count').zero? && + Array(aggregation.groups).empty? && aggregation.field.nil? + + raise UnsupportedOperatorError, + "#{name} can only be counted: Intercom exposes no aggregate endpoint, and grouping or summing the " \ + 'pages the agent read would answer a fraction of the collection as if it were the whole of it.' + end + + # Intercom takes no order on this listing at all -- there is no parameter + # for one -- so an order asked for and not applied is reported here or + # nowhere. The ascending primary-key sort the agent injects when a request + # names none is not one of those. + def warn_ignored_sort(sort) + clauses = Array(sort) + return if clauses.empty? || default_pk_sort?(clauses) + + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] #{name} was asked to sort on " \ + "#{clauses.map { |clause| clause[:field] || clause["field"] }.join(", ")}, and Intercom takes no order " \ + 'on this listing. The rows come back in the order the API imposes.' + ) + end + + def default_pk_sort?(clauses) + return false unless clauses.size == 1 + + clause = clauses.first + return false unless (clause[:field] || clause['field']).to_s == primary_key + + ascending = clause.key?(:ascending) ? clause[:ascending] : clause['ascending'] + ascending != false + end + + def warn_truncated_ids(asked) + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] #{name} was asked for #{asked} records by id and read the first " \ + "#{MAX_ID_READS}: Intercom reads them one request each. The result is truncated." + ) + end + + def warn_truncated_lookup(params) + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] #{name} looked up #{params.inspect} and Intercom advertised more " \ + 'records than one page holds; the rest is left out. This lookup is meant for a key that names one record.' + ) + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/relations.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/relations.rb index bcb7dc03b..22e1f731e 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/relations.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/relations.rb @@ -28,6 +28,7 @@ module Collections module Relations # rubocop:disable Metrics/ModuleLength ManyToOneSchema = ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema ManyToManySchema = ForestAdminDatasourceToolkit::Schema::Relations::ManyToManySchema + OneToManySchema = ForestAdminDatasourceToolkit::Schema::Relations::OneToManySchema Filter = ForestAdminDatasourceToolkit::Components::Query::Filter Projection = ForestAdminDatasourceToolkit::Components::Query::Projection Operators = ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators @@ -66,6 +67,22 @@ def add_many_to_one(name, foreign_collection:, foreign_key:) is_read_only: true)) end + # The other side of a many-to-one, and the whole point of the 360 degrees: + # a contact's conversations, a company's contacts. The agent serves it by + # listing the target on `origin_key equals `, so the + # target has to be able to answer that condition -- which is what makes + # this declarable here and not everywhere. + # + # Published unfilterable by the agent (`GeneratorField`), so unlike a + # many-to-one it carries no risk of offering a filter this datasource + # would then refuse. + def add_one_to_many(name, foreign_collection:, origin_key:) + add_field(name, OneToManySchema.new(foreign_collection: foreign_collection, + origin_key: origin_key, + origin_key_target: 'id', + is_read_only: true)) + end + def add_many_to_many(name, foreign_collection:, through_collection:, origin_key:, foreign_key:) add_field(name, ManyToManySchema.new(foreign_collection: foreign_collection, through_collection: through_collection, @@ -224,13 +241,25 @@ def check_relation_filterable!(_leaf, _relation); end # over every record Intercom holds rather than over a page of them. def matching_ids(caller, relation, leaf) target = relation.foreign_key_target + filter = Filter.new(condition_tree: leaf, page: match_page) foreign_collection(relation) - .list(caller, Filter.new(condition_tree: leaf), Projection.new([target])) + .list(caller, filter, Projection.new([target])) .filter_map { |row| row[target] } .uniq end + # How much of the target a relation condition may read. Unbounded here: + # the tier that answers one in memory holds every record already, and + # cutting the read short would drop ids its `in` can carry for free. + # + # The tier whose target is a page of something larger overrides it -- see + # `CursorCollection`. Resolving `contact:email contains "@"` against a + # workspace's whole contact list, to then refuse the fan-out it comes to, + # would spend a full cursor walk on a filter that was never going to be + # answered. + def match_page = nil + # The target as the datasource holds it, undecorated -- so a permission # scope or a segment defined on the target does not narrow what a relation # resolves. That is how a native datasource behaves too: it joins the table 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 f071cd37c..a5abc7587 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 @@ -46,9 +46,8 @@ def max_page_size = MAX_TICKETS_PER_PAGE # 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 searchable_only? = true + def match_all_query = MATCH_EVERY_TICKET def enrich(records, rows, projection) wanted = Array(projection).map(&:to_s) @@ -110,14 +109,17 @@ def define_type_columns add_column('ticket_type_name', 'String') end - # The four reference collections a ticket points at. Every target is read - # whole in one request, so a relation resolves for a page at the price of a - # single read. + # The reference collections a ticket points at, and the contact who opened + # it. Every reference target is read whole in one request, so those + # relations resolve for a page at the price of a single read; the contact + # is read from `/contacts/search`, one request for the page as well. # - # Only two of them can be filtered *through*: `/tickets/search` takes a - # filter on `admin_assignee_id`, `team_assignee_id` and `ticket_type_id`, - # and none on a state id -- which the refusal names when a filter reaches - # for it, rather than letting the interface offer what the endpoint drops. + # Which of them can be filtered *through* is the measured table's + # business, not this method's: `/tickets/search` takes a filter on + # `admin_assignee_id`, `team_assignee_id` and `ticket_type_id`, none on a + # state id, and `contact_ids` is a `spec` row the probe has yet to + # confirm. Where the endpoint filters nothing, the traversal is refused by + # name rather than left for the interface to offer and the API to drop. def define_relations add_many_to_one('admin_assignee', foreign_collection: 'IntercomAdmin', foreign_key: 'admin_assignee_id') add_many_to_one('team_assignee', foreign_collection: 'IntercomTeam', foreign_key: 'team_assignee_id') @@ -125,6 +127,7 @@ def define_relations add_many_to_one('previous_state', foreign_collection: 'IntercomTicketState', foreign_key: 'previous_state_id') add_many_to_one('ticket_type', foreign_collection: 'IntercomTicketType', foreign_key: 'ticket_type_id') + add_many_to_one('contact', foreign_collection: 'IntercomContact', foreign_key: 'contact_id') end # The attribute columns of every ticket type, in union. Read at boot by diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/datasource.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/datasource.rb index c2a4c2bf0..6a689c6e3 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/datasource.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/datasource.rb @@ -36,17 +36,29 @@ def register_collections add_collection(Collections::TeamMembership.new(self)) add_collection(Collections::TicketType.new(self)) add_collection(Collections::TicketState.new(self)) + # Contacts and Companies before the collections that point at them, so a + # relation is declared next to a target the datasource already holds. + # Each carries the custom attributes its workspace defines, read at boot: + # they cannot be discovered from a payload, a contact carrying the values + # of the attributes it happens to have been given. + add_collection(Collections::Contact.new(self, attributes: model_attributes('contact'))) + add_collection(Collections::Company.new(self, attributes: model_attributes('company'))) add_collection(Collections::Conversation.new(self)) - # The one boot-time read of the datasource: the attributes a workspace - # defines on its ticket types, which are columns of the Tickets collection - # and cannot be discovered from a ticket payload -- a ticket carries the - # values of its own type only. It degrades to no attribute column rather - # than to a failed boot. + # The attributes a workspace defines on its ticket types, which are + # columns of the Tickets collection and cannot be discovered from a ticket + # payload either -- a ticket carries the values of its own type only. add_collection(Collections::Ticket.new(self, attributes: ticket_attributes)) end + # The three boot-time reads of the datasource. Each degrades to no attribute + # column rather than to a failed boot: a token missing a permission costs + # the columns it could not read, never the agent. def ticket_attributes Schema::TicketAttributesIntrospector.new(@client).attributes end + + def model_attributes(model) + Schema::DataAttributesIntrospector.new(@client, model: model).attributes + 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 554b6d175..9f8f9aed8 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 @@ -31,8 +31,13 @@ module SearchFields # `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 + # `sortable` is the one thing here Intercom answers on a single endpoint: + # `/contacts/search` takes a `sort`, the other two take one and ignore it + # without a word. Absent reads as false, so a table saying nothing leaves + # a column unsortable rather than promising an order. + Field = Struct.new(:column, :field, :type, :operators, :source, :sortable, keyword_init: true) do def measured? = source == 'measured' + def sortable? = sortable == true end # A column that stays unfilterable, and why. The reason travels into the @@ -43,7 +48,7 @@ def measured? = source == 'measured' end Endpoint = Struct.new(:name, :path, :measured_at, :fields, :refused, :candidates, :ticket_attributes, - keyword_init: true) do + :custom_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? @@ -51,6 +56,7 @@ def measured? = !measured_at.nil? def field(column) = fields[column] def refusal(column) = refused[column] def filterable_columns = fields.keys + def sortable_columns = fields.values.select(&:sortable?).map(&:column) def unmeasured_fields = fields.values.reject(&:measured?) end @@ -84,14 +90,16 @@ def endpoint(name, definition) fields: fields(name, definition['fields']), refused: refusals(name, definition['refused']), candidates: Array(definition['candidates']).freeze, - ticket_attributes: definition['ticket_attributes'] + ticket_attributes: definition['ticket_attributes'], + custom_attributes: definition['custom_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 + operators: Array(row['operators']).freeze, source: row.fetch('source'), + sortable: row.fetch('sortable', false)).freeze validate_field!(endpoint, field) [column, field] @@ -112,6 +120,16 @@ def validate_field!(endpoint, field) validate_source!(endpoint, field.column, field) validate_type!(endpoint, field) validate_operators!(endpoint, field) + validate_sortable!(endpoint, field) + end + + # Anything but a boolean, `"true"` above all: YAML reads it as a string, + # which is truthy in Ruby and would publish a sortable column out of a + # typo the file cannot otherwise show. + def validate_sortable!(endpoint, field) + return if [true, false].include?(field.sortable) + + malformed!(endpoint, field.column, "sortable #{field.sortable.inspect} is neither true nor false") end def validate_type!(endpoint, field) 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 d2bb0ad2b..414de6c6a 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 @@ -178,19 +178,26 @@ endpoints: type: boolean operators: ['='] source: spec + # The foreign key of the `contact` relation, and the reason lot 4 could + # turn the denormalized contact columns into one: Intercom matches a + # conversation against one of the contacts it carries. The column is + # singular and the wire field plural on purpose -- the row names its first + # contact, the endpoint asks "is this contact one of yours". + # + # `!=` is deliberately absent: on a field holding a list, "not that + # contact" is a question Intercom's DSL does not clearly answer, and the + # one thing this table may not do is publish a filter whose meaning is a + # guess. + contact_id: + field: contact_ids + type: string + operators: ['='] + source: measured # 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: - 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 @@ -216,13 +223,10 @@ endpoints: 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. + Read from the Contacts endpoint, not from the conversation. Filter + through the `contact` relation, which resolves against + `/contacts/search` and rewrites onto the contact id -- or on + `source_author_email`, which is on the conversation itself. source: spec contact_count: reason: Counted by the agent from the contacts the payload carries. @@ -305,15 +309,18 @@ endpoints: type: date operators: ['>', '<', '>=', '<=', '=', '!='] source: measured + # The foreign key of the `contact` relation. `spec` rather than + # `measured`, and the difference is visible to an operator: it is what + # decides whether the contacts of a ticket can be filtered at all, and the + # probe has not run on this endpoint. If Intercom refuses it, this row + # moves to the refused table and the relation stays navigable without + # being filterable -- the way `state` already is. + contact_id: + field: contact_ids + type: string + operators: ['='] + source: spec 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 @@ -380,9 +387,229 @@ endpoints: - state_id - is_shared - previous_state_id - - contact_id + - company_ids - source.author.email - source.subject - source.body - title - description + + # The one endpoint of the whole API that sorts. `sortable: true` is what a + # column reads to publish `isSortable`, and it appears nowhere else in this + # file on purpose: `/conversations/search` accepts a `sort` and ignores it + # without a word (measured), so a row that stayed silent there would be a + # column promising an order Intercom never applies. + # + # The set is deliberately narrower than what the documentation implies. It + # says the results may be sorted by any attribute; nothing has been measured, + # and a sort Intercom refuses is a list view that fails rather than one that + # comes back unordered. These are the fields an ops team orders a contact list + # by, and the probe is what may widen the set. + contacts: + path: contacts/search + measured_at: null + fields: + # Measured rather than taken from the documentation: lot 1 reads the + # contacts of a page through `id IN [...]` on this endpoint, which is the + # membership operator every other field refuses. + id: + field: id + type: string + operators: ['=', 'IN'] + source: measured + role: + field: role + type: string + operators: ['=', '!='] + source: spec + name: + field: name + type: text + operators: ['=', '!=', '~', '!~', '^', '$'] + source: spec + sortable: true + email: + field: email + type: text + operators: ['=', '!=', '~', '!~', '^', '$'] + source: spec + sortable: true + # Derived by the agent from the address, and filtered by Intercom all the + # same: the endpoint carries a field of its own for it, which is what + # makes "everyone at this customer" a filter rather than a wildcard. + email_domain: + field: email_domain + type: string + operators: ['=', '!='] + source: spec + phone: + field: phone + type: text + operators: ['=', '!=', '~', '!~', '^', '$'] + source: spec + external_id: + field: external_id + type: string + operators: ['=', '!='] + source: spec + owner_id: + field: owner_id + type: string + operators: ['=', '!='] + source: spec + # Measured on 25 August 2026, API 2.16, and this is the asymmetry the + # per-endpoint table exists for: `/conversations/search` and + # `/tickets/search` accept `>=`, `<=` and `!=` on a date where this + # endpoint answers `data_invalid`. `IN` is refused on a date everywhere. + # + # Nothing is lost that an operator can see: a Date column publishes the + # two bounds alone whatever this table allows -- see the operator table -- + # and the toolkit derives `before`, `after`, `today` and the whole + # `previous_*` family from them. + created_at: + field: created_at + type: date + operators: ['>', '<'] + source: measured + sortable: true + updated_at: + field: updated_at + type: date + operators: ['>', '<'] + source: measured + sortable: true + signed_up_at: + field: signed_up_at + type: date + operators: ['>', '<'] + source: spec + sortable: true + last_seen_at: + field: last_seen_at + type: date + operators: ['>', '<'] + source: spec + sortable: true + last_contacted_at: + field: last_contacted_at + type: date + operators: ['>', '<'] + source: spec + sortable: true + last_replied_at: + field: last_replied_at + type: date + operators: ['>', '<'] + source: spec + sortable: true + last_email_opened_at: + field: last_email_opened_at + type: date + operators: ['>', '<'] + source: spec + last_email_clicked_at: + field: last_email_clicked_at + type: date + operators: ['>', '<'] + source: spec + unsubscribed_from_emails: + field: unsubscribed_from_emails + type: boolean + operators: ['='] + source: spec + has_hard_bounced: + field: has_hard_bounced + type: boolean + operators: ['='] + source: spec + marked_email_as_spam: + field: marked_email_as_spam + type: boolean + operators: ['='] + source: spec + language_override: + field: language_override + type: string + operators: ['=', '!='] + source: spec + browser: + field: browser + type: string + operators: ['=', '!='] + source: spec + browser_language: + field: browser_language + type: string + operators: ['=', '!='] + source: spec + os: + field: os + type: string + operators: ['=', '!='] + source: spec + location_country: + field: location.country + type: string + operators: ['=', '!='] + source: spec + location_region: + field: location.region + type: string + operators: ['=', '!='] + source: spec + location_city: + field: location.city + type: string + operators: ['=', '!='] + source: spec + refused: + avatar: + reason: >- + The url of the contact's picture. Nothing filters an image, and + Intercom's search does not carry the field. + source: spec + company_id: + reason: >- + The first of the companies the contact belongs to, read off the + payload. `/contacts/search` filters no company field, so a company + list is reached from the company side -- open the company and read + its contacts. + source: spec + company_count: + reason: Counted by the agent from the companies the payload carries. + source: spec + session_count: + reason: >- + On the contact payload and not in the search DSL. Filter on + `last_seen_at` instead, which the endpoint does take. + source: spec + # Contact custom attributes are filtered as `custom_attributes.{name}` -- + # by name, unlike a ticket attribute, which carries a different id per + # ticket type. So the obstacle is not R7 here, it is measurement: the + # operator set Intercom answers on a custom attribute of each data type has + # not been probed, and this package does not publish a filter it has not + # seen work. They ship typed and display-only, and the probe is what turns + # that around. + custom_attributes: + filterable: false + reason: >- + Contact custom attributes are published for reading only: which + operators Intercom answers on `custom_attributes.{name}` has not been + measured on a real workspace, and this datasource does not offer a + filter it has not seen work. Filter on a native field instead. + source: spec + candidates: + - tag_ids + - segment_id + - company_id + - companies.id + - companies.name + - android_app_name + - android_last_seen_at + - ios_app_name + - ios_last_seen_at + - utm_source + - utm_campaign + - utm_medium + - referrer + - session_count diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/schema/data_attributes_introspector.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/schema/data_attributes_introspector.rb new file mode 100644 index 000000000..5a2d4422e --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/schema/data_attributes_introspector.rb @@ -0,0 +1,119 @@ +module ForestAdminDatasourceIntercom + module Schema + # The attributes a workspace defines on its contacts and on its companies, + # read once while the datasource is being constructed. + # + # Unlike a ticket attribute, one of these is declared **per model** rather + # than per ticket type, so the union is the whole set and a column maps onto + # exactly one Intercom attribute -- the ambiguity that keeps ticket + # attributes display-only (R7) does not arise here. What keeps these + # display-only is narrower and temporary: which operators Intercom answers + # on `custom_attributes.{name}` has not been measured, and this package + # publishes no filter it has not seen work. + # + # `api_writable` is read and carried although every column of this lot is + # published read-only. It costs nothing now and it is exactly what lot 4b + # needs to tell an attribute it may write from one Intercom fills in + # itself -- re-reading it later would be a second boot-time round trip. + class DataAttributesIntrospector + # Intercom's attribute data types, mapped onto what Forest can render. + COLUMN_TYPES = { + 'string' => 'String', 'integer' => 'Number', 'float' => 'Number', 'decimal' => 'Number', + 'boolean' => 'Boolean', 'date' => 'Date', 'datetime' => 'Date' + }.freeze + + DEFAULT_COLUMN_TYPE = 'String'.freeze + + # What a column name may not contain, and it has nothing to do with + # Intercom: Forest lists the fields of a request in a comma-separated + # query parameter and names a field through a relation with a colon. A + # workspace names its attributes in free text, and a comma in there splits + # the projection into fields no collection has. + UNSAFE_IN_A_COLUMN_NAME = /[,:]/ + + # `name` is the key `custom_attributes` uses, `column_name` the one the + # schema publishes; they differ whenever the workspace's own name cannot + # travel through Forest's query string. + Attribute = Struct.new(:name, :column_name, :column_type, :data_type, :api_writable, keyword_init: true) + + def initialize(client, model:) + @client = client + @model = model + end + + # Degrades to nothing rather than to a failure: a token without the + # permission on this model costs the custom-attribute columns, never the + # boot of the agent. + def attributes + @attributes ||= build + rescue APIError => e + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] could not read the #{@model} attributes (HTTP #{e.status || "-"}); " \ + 'the collection boots without its custom-attribute columns.' + ) + @attributes = [] + end + + private + + def build + # Read on the boot connection: this happens while Rails is starting, and + # a slow Intercom must not turn that into minutes the operator sits + # through. + @client.fetch_all('data_attributes', params: { 'model' => @model }, boot: true) + .each_with_object({}) { |definition, union| collect(definition, union) } + .values + end + + # The standard attributes are left out: they are columns this datasource + # declares by hand, with the filters the search table measured, and + # publishing them a second time under their `custom_attributes` name would + # show one fact twice -- the unfilterable copy winning nothing. + def collect(definition, union) + return unless definition.is_a?(Hash) && definition['custom'] && !definition['archived'] + + name = definition['name'].to_s + return if name.empty? + + column = column_name_for(name) + return if column.empty? + + add(union, name, column, definition) + end + + def add(union, name, column, definition) + entry = union[column] + return union[column] = attribute_from(name, column, definition) if entry.nil? + return if entry.name == name + + warn_collision(name, entry.name, column) + end + + def attribute_from(name, column, definition) + Attribute.new(name: name, column_name: column, column_type: column_type_for(definition), + data_type: definition['data_type'], api_writable: definition['api_writable'] == true) + end + + # Intercom hands these back HTML-escaped -- `Ce que j'ai vérifié` -- + # which is an artefact of where they were typed, not part of the name. + def column_name_for(name) + CGI.unescapeHTML(name).gsub(UNSAFE_IN_A_COLUMN_NAME, ' ').squeeze(' ').strip + end + + def warn_collision(name, kept, column) + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] the #{@model} attribute #{name.inspect} is left out: it reads as the " \ + "column #{column.inspect}, which #{kept.inspect} already carries. Rename one of them in Intercom to " \ + 'publish both.' + ) + end + + # An unknown data type reads as a string rather than being dropped: + # showing the value Intercom sent beats hiding a column because its type + # is new. + def column_type_for(definition) + COLUMN_TYPES.fetch(definition['data_type'].to_s, DEFAULT_COLUMN_TYPE) + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb new file mode 100644 index 000000000..6151d1298 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb @@ -0,0 +1,354 @@ +module ForestAdminDatasourceIntercom + # The offset tier is exercised through Companies, the one collection Intercom + # paginates that way -- and the only one it does not search at all. + RSpec.describe Collections::Company do + subject(:collection) { datasource.get_collection('IntercomCompany') } + + let(:datasource) { Datasource.new(access_token: 's3cr3t', rate_limiter: nil) } + let(:base) { datasource.configuration.url } + let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + + def json(payload, status = 200) + { status: status, body: payload.to_json, headers: { 'Content-Type' => 'application/json' } } + end + + def leaf(field, operator, value = nil) + ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf + .new(field, operator, value) + end + + def branch(aggregator, *conditions) + ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeBranch + .new(aggregator, conditions) + end + + def filter(condition_tree: nil, page: nil, sort: nil) + ForestAdminDatasourceToolkit::Components::Query::Filter.new(condition_tree: condition_tree, page: page, + sort: sort) + end + + def page(offset, limit) + ForestAdminDatasourceToolkit::Components::Query::Page.new(offset: offset, limit: limit) + end + + def sort(*clauses) + ForestAdminDatasourceToolkit::Components::Query::Sort.new(clauses) + end + + def count(condition_tree: nil) + collection.aggregate(nil, filter(condition_tree: condition_tree), + ForestAdminDatasourceToolkit::Components::Query::Aggregation.new(operation: 'Count')) + end + + def company(id, overrides = {}) + { 'type' => 'company', 'id' => id, 'company_id' => "erp-#{id}", 'name' => "Company #{id}", + 'plan' => { 'type' => 'plan', 'id' => '9', 'name' => 'Paid' }, 'size' => 85, + 'industry' => 'Manufacturing', 'website' => 'https://acme.test', 'monthly_spend' => 49, + 'session_count' => 26, 'user_count' => 10, 'created_at' => 1_700_000_000, + 'updated_at' => 1_700_000_600, 'last_request_at' => 1_700_000_900, + 'remote_created_at' => 1_394_531_169, 'custom_attributes' => {} }.merge(overrides) + end + + def stub_page(*companies, page: 1, per_page: 15, total_pages: 1, total: nil) + stub_request(:post, "#{base}/companies/list") + .with(query: { 'page' => page.to_s, 'per_page' => per_page.to_s }) + .to_return(json('type' => 'list', 'data' => companies, 'total_count' => total || companies.size, + 'pages' => { 'type' => 'pages', 'page' => page, 'per_page' => per_page, + 'total_pages' => total_pages })) + end + + def rows(projection = %w[id], **options) + collection.list(nil, filter(**options), projection) + end + + describe 'schema' do + it 'is named IntercomCompany' do + expect(collection.name).to eq('IntercomCompany') + end + + it 'publishes the columns of an account' do + expect(collection.fields.keys) + .to include('id', 'company_id', 'name', 'plan_name', 'size', 'industry', 'website', + 'monthly_spend', 'user_count', 'session_count', 'created_at', 'remote_created_at') + end + + it 'publishes every column read-only and unsortable' do + columns = collection.fields.values.grep(ForestAdminDatasourceToolkit::Schema::ColumnSchema) + + expect(columns.map(&:is_read_only).uniq).to eq([true]) + expect(columns.map(&:is_sortable).uniq).to eq([false]) + end + + # Four lookups is what `GET /companies` answers, and two of them name a + # column of this collection. A tag and a segment are collections of their + # own, and filtering by them belongs with the lot that adds them. + it 'offers a filter on the two keys Intercom looks a company up by' do + expect(collection.fields['name'].filter_operators).to eq(%w[equal]) + expect(collection.fields['company_id'].filter_operators).to eq(%w[equal]) + expect(collection.fields['id'].filter_operators).to eq(%w[equal in]) + end + + it 'offers no filter on anything else' do + %w[plan_name size industry website monthly_spend user_count created_at].each do |column| + expect(collection.fields[column].filter_operators).to be_empty, "#{column} advertises a filter" + end + end + + it 'declares the contacts of the account' do + expect(collection.fields['contacts'].origin_key).to eq('company_id') + end + + it 'is countable, total_count being exact on every answer' do + expect(collection.is_countable?).to be(true) + end + end + + describe 'the custom attributes' do + before do + stub_data_attributes('company', + { 'name' => 'arr', 'data_type' => 'float', 'custom' => true, + 'api_writable' => true, 'archived' => false }) + end + + it 'publishes one column per attribute, typed and display-only' do + expect(collection.fields['arr'].column_type).to eq('Number') + expect(collection.fields['arr'].filter_operators).to be_empty + end + + it 'reads its value off the payload' do + stub_page(company('co1', 'custom_attributes' => { 'arr' => 12_000 })) + + expect(rows(%w[id arr], page: page(0, 15))).to eq([{ 'id' => 'co1', 'arr' => 12_000 }]) + end + end + + describe 'pagination by offset' do + # The one place R1 does not apply: Intercom counts pages, which is what a + # list view asks for. No cursor walked, no page read to be thrown away. + it 'asks for the page the window names, in one request' do + stub_page(company('co1'), page: 3, per_page: 15, total_pages: 3) + + expect(rows(%w[id], page: page(30, 15))).to eq([{ 'id' => 'co1' }]) + expect(WebMock).to have_requested(:post, "#{base}/companies/list") + .with(query: { 'page' => '3', 'per_page' => '15' }).once + end + + # An offset that does not fall on a page boundary is served exactly, by + # reading the page it lands in and the next -- never by rounding the + # window to something the API likes better. + it 'reads across two pages when the window straddles them' do + stub_page(company('a'), company('b'), page: 1, per_page: 2, total_pages: 3) + stub_page(company('c'), company('d'), page: 2, per_page: 2, total_pages: 3) + + expect(rows(%w[id], page: page(1, 2)).map { |row| row['id'] }).to eq(%w[b c]) + end + + it 'stops at the last page rather than asking for one past it' do + stub_page(company('a'), page: 1, per_page: 15, total_pages: 1) + + expect(rows(%w[id], page: page(0, 15)).size).to eq(1) + expect(WebMock).not_to have_requested(:post, "#{base}/companies/list") + .with(query: { 'page' => '2', 'per_page' => '15' }) + end + + it 'stops on a page Intercom answers empty' do + stub_page(page: 1, per_page: 2, total_pages: 9) + + expect(rows(%w[id], page: page(0, 2))).to be_empty + end + + # A read naming no window -- a segment, a customizer -- is the only one + # that can run long, and it is the only one this bounds. + it 'reads full pages when the read names no window' do + stub_page(company('a'), page: 1, per_page: 150, total_pages: 1) + + expect(rows(%w[id]).map { |row| row['id'] }).to eq(%w[a]) + end + + it 'stops such a read after the pages it allows, and says so' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + (1..10).each { |number| stub_page(company("c#{number}"), page: number, per_page: 150, total_pages: 99) } + + expect(rows(%w[id]).size).to eq(10) + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/Stopped reading IntercomCompany/) + end + + it 'counts what Intercom counted, in one request' do + stub_page(company('a'), page: 1, per_page: 1, total_pages: 40, total: 597) + + expect(count).to eq([{ 'group' => {}, 'value' => 597 }]) + end + + # Counting the pages read would answer a fraction of the collection as if + # it were the whole of it. + it 'refuses to count a listing Intercom answered without a total' do + stub_request(:post, "#{base}/companies/list").with(query: hash_including({})) + .to_return(json('type' => 'list', 'data' => [company('a')], + 'pages' => {})) + + expect { count }.to raise_error(UnsupportedOperatorError, /cannot be counted/) + end + end + + describe 'the four lookups, and everything past them' do + it 'reads a record detail through its own endpoint' do + stub_request(:get, "#{base}/companies/co1").to_return(json(company('co1'))) + + expect(rows(%w[id], condition_tree: leaf('id', operators::EQUAL, 'co1'))).to eq([{ 'id' => 'co1' }]) + end + + it 'reads a set of ids one request each' do + stub_request(:get, "#{base}/companies/co1").to_return(json(company('co1'))) + stub_request(:get, "#{base}/companies/co2").to_return(json(company('co2'))) + + expect(rows(%w[id], condition_tree: leaf('id', operators::IN, %w[co1 co2])).map { |row| row['id'] }) + .to eq(%w[co1 co2]) + end + + it 'reads a company the token can no longer reach as no record' do + stub_request(:get, "#{base}/companies/co1").to_return(json({ 'type' => 'error.list' }, 404)) + + expect(rows(%w[id], condition_tree: leaf('id', operators::EQUAL, 'co1'))).to be_empty + end + + it 'raises on a failure that is not a missing record' do + stub_request(:get, "#{base}/companies/co1").to_return(json({ 'type' => 'error.list' }, 500)) + + expect { rows(%w[id], condition_tree: leaf('id', operators::EQUAL, 'co1')) }.to raise_error(APIError) + end + + it 'truncates a set of ids larger than it will read, and says so' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_request(:get, %r{#{base}/companies/co\d+}).to_return(json(company('co1'))) + + rows(%w[id], condition_tree: leaf('id', operators::IN, (1..30).map { |n| "co#{n}" })) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/asked for 30 records by id/) + end + + it 'counts the records the ids named' do + stub_request(:get, "#{base}/companies/co1").to_return(json(company('co1'))) + + expect(count(condition_tree: leaf('id', operators::EQUAL, 'co1'))).to eq([{ 'group' => {}, 'value' => 1 }]) + end + + # `GET /companies?name=` answers the company itself where a listing would + # answer an envelope: a record is read as a page of one rather than as a + # shape every caller has to test for. + it 'looks a company up by name, and reads the record Intercom answers' do + stub_request(:get, "#{base}/companies").with(query: { 'name' => 'Acme' }) + .to_return(json(company('co1'))) + + expect(rows(%w[id], condition_tree: leaf('name', operators::EQUAL, 'Acme'))).to eq([{ 'id' => 'co1' }]) + end + + it 'looks one up by the identifier the workspace gave it' do + stub_request(:get, "#{base}/companies").with(query: { 'company_id' => 'erp-co1' }) + .to_return(json('type' => 'list', 'data' => [company('co1')], + 'total_count' => 1, 'pages' => {})) + + expect(rows(%w[id], condition_tree: leaf('company_id', operators::EQUAL, 'erp-co1'))) + .to eq([{ 'id' => 'co1' }]) + end + + it 'counts what a lookup found' do + stub_request(:get, "#{base}/companies").with(query: { 'name' => 'Acme' }) + .to_return(json(company('co1'))) + + expect(count(condition_tree: leaf('name', operators::EQUAL, 'Acme'))) + .to eq([{ 'group' => {}, 'value' => 1 }]) + end + + it 'reports a lookup Intercom answered with more than one page' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_request(:get, "#{base}/companies").with(query: { 'name' => 'Acme' }) + .to_return(json('type' => 'list', 'data' => [company('co1')], + 'pages' => { 'next' => { 'starting_after' => 'zzz' } })) + + rows(%w[id], condition_tree: leaf('name', operators::EQUAL, 'Acme')) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/advertised more records/) + end + + # A filter this collection cannot look up is refused by name. A page + # served in answer to a filter it ignored is the one failure this + # datasource is built to avoid. + it 'refuses a filter on a column Intercom does not look up' do + expect { rows(%w[id], condition_tree: leaf('industry', operators::EQUAL, 'Manufacturing')) } + .to raise_error(UnsupportedOperatorError, /cannot filter "industry".*name, company_id alone/m) + end + + it 'refuses an operator the lookup has no equivalent for' do + expect { rows(%w[id], condition_tree: leaf('name', operators::CONTAINS, 'Acm')) } + .to raise_error(UnsupportedOperatorError, /cannot filter "name"/) + end + + it 'refuses a combination, the lookup answering one value at a time' do + expect do + rows(%w[id], condition_tree: branch('And', leaf('name', operators::EQUAL, 'Acme'), + leaf('company_id', operators::EQUAL, 'erp-co1'))) + end.to raise_error(UnsupportedOperatorError, /one exact value at a time/) + end + + it 'refuses to count what it refuses to list' do + expect { count(condition_tree: leaf('industry', operators::EQUAL, 'Manufacturing')) } + .to raise_error(UnsupportedOperatorError, /cannot filter "industry"/) + end + end + + describe 'what it will not do' do + it 'refuses to group, Intercom exposing no aggregate endpoint' do + expect do + collection.aggregate(nil, filter, ForestAdminDatasourceToolkit::Components::Query::Aggregation + .new(operation: 'Count', groups: [{ field: 'industry' }])) + end.to raise_error(UnsupportedOperatorError, /can only be counted/) + end + + # There is no order parameter on this listing at all, so an order asked + # for and not applied is reported here or nowhere. + it 'reports an order it cannot apply' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_page(company('a'), page: 1, per_page: 15) + + rows(%w[id], page: page(0, 15), sort: sort({ field: 'name', ascending: true })) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/takes no order on this listing/) + end + + it 'says nothing of the ascending primary-key order the agent injects' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_page(company('a'), page: 1, per_page: 15) + + rows(%w[id], page: page(0, 15), sort: sort({ field: 'id', ascending: true })) + + expect(ForestAdminDatasourceIntercom.logger).not_to have_received(:warn) + end + + it 'reports an explicit descending order on the key, which it cannot apply either' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_page(company('a'), page: 1, per_page: 15) + + rows(%w[id], page: page(0, 15), sort: sort({ field: 'id', ascending: false })) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/sort on id/) + end + end + + describe 'the row' do + it 'flattens the payload, the plan read off the object that carries it' do + stub_page(company('co1'), page: 1, per_page: 15) + + expect(rows(nil, page: page(0, 15)).first) + .to include('id' => 'co1', 'company_id' => 'erp-co1', 'name' => 'Company co1', 'plan_name' => 'Paid', + 'size' => 85, 'monthly_spend' => 49, 'user_count' => 10, + 'created_at' => '2023-11-14T22:13:20Z', 'remote_created_at' => '2014-03-11T09:46:09Z') + end + + it 'reads a company carrying no plan without failing' do + stub_page(company('co1', 'plan' => nil), page: 1, per_page: 15) + + expect(rows(nil, page: page(0, 15)).first).to include('plan_name' => nil) + end + end + end +end diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb new file mode 100644 index 000000000..7111bb263 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb @@ -0,0 +1,445 @@ +module ForestAdminDatasourceIntercom + RSpec.describe Collections::Contact do + subject(:collection) { datasource.get_collection('IntercomContact') } + + let(:datasource) { Datasource.new(access_token: 's3cr3t', rate_limiter: nil) } + let(:base) { datasource.configuration.url } + let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + + def json(payload, status = 200) + { status: status, body: payload.to_json, headers: { 'Content-Type' => 'application/json' } } + end + + def leaf(field, operator, value = nil) + ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeLeaf + .new(field, operator, value) + end + + def branch(aggregator, *conditions) + ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes::ConditionTreeBranch + .new(aggregator, conditions) + end + + def filter(condition_tree: nil, page: nil, sort: nil, search: nil) + ForestAdminDatasourceToolkit::Components::Query::Filter.new(condition_tree: condition_tree, page: page, + sort: sort, search: search) + end + + def page(offset, limit) + ForestAdminDatasourceToolkit::Components::Query::Page.new(offset: offset, limit: limit) + end + + def sort(*clauses) + ForestAdminDatasourceToolkit::Components::Query::Sort.new(clauses) + end + + def contact(id, overrides = {}) + { 'type' => 'contact', 'id' => id, 'role' => 'user', 'name' => "Contact #{id}", + 'email' => "#{id}@acme.test", 'phone' => nil, 'external_id' => "ext-#{id}", 'owner_id' => 493_881, + 'created_at' => 1_700_000_000, 'updated_at' => 1_700_000_600, 'session_count' => 3, + 'unsubscribed_from_emails' => false, 'has_hard_bounced' => false, 'marked_email_as_spam' => false, + 'browser' => 'chrome', 'browser_language' => 'fr', 'os' => 'OS X', 'language_override' => nil, + 'location' => { 'type' => 'location', 'country' => 'France', 'region' => 'IdF', 'city' => 'Paris' }, + 'companies' => { 'type' => 'list', 'data' => [{ 'type' => 'company', 'id' => 'co1' }], + 'total_count' => 2 }, + 'custom_attributes' => {} }.merge(overrides) + end + + def stub_list(*contacts, cursor: nil) + pages = cursor ? { 'next' => { 'starting_after' => cursor } } : {} + stub_request(:get, "#{base}/contacts").with(query: hash_including({})) + .to_return(json('type' => 'list', 'data' => contacts, + 'total_count' => contacts.size, 'pages' => pages)) + end + + def stub_search(*contacts, total: nil) + stub_request(:post, "#{base}/contacts/search") + .to_return(json('type' => 'list', 'data' => contacts, 'total_count' => total || contacts.size, + 'pages' => {})) + end + + def rows(projection = %w[id], **options) + collection.list(nil, filter(**options), projection) + end + + describe 'schema' do + it 'is named IntercomContact' do + expect(collection.name).to eq('IntercomContact') + end + + it 'publishes the columns of a contact, the account it belongs to included' do + expect(collection.fields.keys) + .to include('id', 'role', 'name', 'email', 'email_domain', 'phone', 'external_id', 'avatar', + 'owner_id', 'company_id', 'company_count', 'session_count', 'created_at', + 'last_seen_at', 'unsubscribed_from_emails', 'location_country') + end + + it 'publishes every column read-only, this lot writing nothing' do + columns = collection.fields.values.grep(ForestAdminDatasourceToolkit::Schema::ColumnSchema) + + expect(columns.map(&:is_read_only).uniq).to eq([true]) + end + + # The measured asymmetry: `/contacts/search` refuses `>=`, `<=`, `!=` and + # `IN` on a date where the other two endpoints take them. A Date column + # publishes the two bounds alone anyway -- declaring `equal` would make + # the toolkit republish `in`, which its own validator then refuses + # (PRD-989) -- so what the restriction really has to guarantee is that + # nothing wider reaches the wire. + it 'offers the two bounds on a date, and nothing the endpoint refuses' do + expect(collection.fields['created_at'].filter_operators).to eq(%w[greater_than less_than]) + expect(collection.fields['last_seen_at'].filter_operators).to eq(%w[greater_than less_than]) + end + + it 'offers on a text column exactly what the table measured' do + expect(collection.fields['email'].filter_operators) + .to eq(%w[equal not_equal contains i_contains not_contains starts_with ends_with]) + expect(collection.fields['role'].filter_operators).to eq(%w[equal not_equal]) + end + + # A column the table does not carry advertises nothing, which is how a + # refusal is spelled in a schema. + it 'advertises no filter on a column the endpoint does not filter' do + %w[avatar company_id company_count session_count].each do |column| + expect(collection.fields[column].filter_operators).to be_empty, "#{column} advertises a filter" + end + end + + # The one collection of the whole API Intercom sorts, and the reason this + # tier reads a `sortable` flag off the measured table at all. + it 'is sortable on the columns the table measured, and on no others' do + sortable = collection.fields.select { |_, f| f.respond_to?(:is_sortable) && f.is_sortable }.keys + + expect(sortable).to contain_exactly('name', 'email', 'created_at', 'updated_at', 'signed_up_at', + 'last_seen_at', 'last_contacted_at', 'last_replied_at') + end + + it 'declares the relations the 360 degrees is walked through' do + expect(collection.fields['owner']).to be_a(ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema) + expect(collection.fields['company']).to be_a(ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema) + expect(collection.fields['conversations']) + .to be_a(ForestAdminDatasourceToolkit::Schema::Relations::OneToManySchema) + expect(collection.fields['tickets'].origin_key).to eq('contact_id') + end + + it 'is countable, total_count being exact on every answer' do + expect(collection.is_countable?).to be(true) + end + + it 'is searchable on the address an ops team types' do + expect(collection.is_searchable?).to be(true) + end + end + + describe 'the custom attributes' do + subject(:collection) { datasource.get_collection('IntercomContact') } + + before do + stub_data_attributes('contact', + { 'name' => 'paid_subscriber', 'data_type' => 'boolean', 'custom' => true, + 'api_writable' => true, 'archived' => false }, + { 'name' => 'email', 'data_type' => 'string', 'custom' => false, + 'api_writable' => false, 'archived' => false }) + end + + it 'publishes one column per custom attribute, typed from the introspection' do + expect(collection.fields['paid_subscriber'].column_type).to eq('Boolean') + end + + # Display-only: which operators Intercom answers on + # `custom_attributes.{name}` has not been measured, and this package + # publishes no filter it has not seen work. + it 'publishes it unfilterable and unsortable' do + expect(collection.fields['paid_subscriber'].filter_operators).to be_empty + expect(collection.fields['paid_subscriber'].is_sortable).to be(false) + end + + it 'reads its value off the payload, nil where the contact carries none' do + stub_list(contact('1', 'custom_attributes' => { 'paid_subscriber' => true }), contact('2')) + + expect(rows(%w[id paid_subscriber])) + .to eq([{ 'id' => '1', 'paid_subscriber' => true }, { 'id' => '2', 'paid_subscriber' => nil }]) + end + end + + describe '#list' do + it 'walks the listing endpoint when nothing is filtered, sorted or searched' do + stub_list(contact('1'), contact('2')) + + expect(rows).to eq([{ 'id' => '1' }, { 'id' => '2' }]) + expect(WebMock).not_to have_requested(:post, "#{base}/contacts/search") + end + + it 'flattens the payload onto the row' do + stub_list(contact('1')) + + expect(rows(nil).first) + .to include('id' => '1', 'role' => 'user', 'email' => '1@acme.test', 'email_domain' => 'acme.test', + 'owner_id' => '493881', 'session_count' => 3, 'created_at' => '2023-11-14T22:13:20Z', + 'location_country' => 'France', 'location_city' => 'Paris') + end + + # A contact belongs to several accounts: the row names the first and says + # how many there are, which Intercom counts itself -- the nested list is + # capped and a contact of twelve accounts must not read as one of ten. + it 'names the first account and takes the count from Intercom' do + stub_list(contact('1')) + + expect(rows(nil).first).to include('company_id' => 'co1', 'company_count' => 2) + end + + it 'counts the accounts it can see when Intercom sends no count' do + stub_list(contact('1', 'companies' => { 'type' => 'list', 'data' => [{ 'id' => 'co1' }] })) + + expect(rows(nil).first).to include('company_count' => 1) + end + + it 'leaves the account columns empty on a contact belonging to none' do + stub_list(contact('1', 'companies' => nil)) + + expect(rows(nil).first).to include('company_id' => nil, 'company_count' => 0) + end + + it 'reads no domain out of an address that has none' do + stub_list(contact('1', 'email' => nil)) + + expect(rows(nil).first).to include('email_domain' => nil) + end + + it 'translates a filter into the search DSL' do + stub_search(contact('1')) + + expect(rows(%w[id], condition_tree: leaf('role', operators::EQUAL, 'lead'))).to eq([{ 'id' => '1' }]) + expect(WebMock).to have_requested(:post, "#{base}/contacts/search") + .with(body: hash_including('query' => { 'field' => 'role', 'operator' => '=', 'value' => 'lead' })) + end + + it 'answers a free-text search on the address, per word' do + stub_search(contact('1')) + + rows(%w[id], search: 'acme.test') + + expect(WebMock).to have_requested(:post, "#{base}/contacts/search") + .with(body: hash_including('query' => { 'field' => 'email', 'operator' => '~', 'value' => 'acme.test' })) + end + end + + describe 'the one collection Intercom sorts' do + # A list view asking for an order has no condition to send, and the + # listing endpoint does not sort: the order is what routes the read + # through the search, with the predicate that matches everything. + it 'sends the order to the search endpoint, with the match-all predicate' do + stub_search(contact('1')) + + rows(%w[id], sort: sort({ field: 'last_seen_at', ascending: false })) + + expect(WebMock).to have_requested(:post, "#{base}/contacts/search") + .with(body: hash_including('query' => described_class::MATCH_EVERY_CONTACT, + 'sort' => { 'field' => 'last_seen_at', 'order' => 'descending' })) + end + + it 'sends it alongside the filter when there is one' do + stub_search(contact('1')) + + rows(%w[id], condition_tree: leaf('role', operators::EQUAL, 'user'), + sort: sort({ field: 'name', ascending: true })) + + expect(WebMock).to have_requested(:post, "#{base}/contacts/search") + .with(body: hash_including('query' => { 'field' => 'role', 'operator' => '=', 'value' => 'user' }, + 'sort' => { 'field' => 'name', 'order' => 'ascending' })) + end + + # The ascending primary-key sort the agent injects when a request names + # none is not an order anybody asked for, and this endpoint does not sort + # on an id anyway. + it 'keeps the listing route for the default primary-key order, and says nothing' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_list(contact('1')) + + rows(%w[id], sort: sort({ field: 'id', ascending: true })) + + expect(WebMock).to have_requested(:get, "#{base}/contacts").with(query: hash_including({})) + expect(ForestAdminDatasourceIntercom.logger).not_to have_received(:warn) + end + + it 'reports an order on a column Intercom does not sort, rather than dropping it' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_list(contact('1')) + + rows(%w[id], sort: sort({ field: 'browser', ascending: true })) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/sort on browser/) + expect(WebMock).not_to have_requested(:post, "#{base}/contacts/search") + end + + # Intercom takes a single `{ field, order }`: honouring the first clause + # alone would order the page by something the operator did not ask for. + it 'reports a composite order rather than honouring half of it' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_list(contact('1')) + + rows(%w[id], sort: sort({ field: 'name', ascending: true }, { field: 'email', ascending: false })) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/sort on name, email/) + end + end + + describe 'reading records by id' do + # One request per hundred ids rather than one per id: this endpoint + # answers `id IN [...]`, which is what makes a related list of contacts + # affordable at all. + it 'reads a set of ids in one request through the search' do + stub_search(contact('1'), contact('2')) + + expect(rows(%w[id], condition_tree: leaf('id', operators::IN, %w[1 2]))) + .to eq([{ 'id' => '1' }, { 'id' => '2' }]) + expect(WebMock).to have_requested(:post, "#{base}/contacts/search") + .with(body: hash_including('query' => { 'field' => 'id', 'operator' => 'IN', 'value' => %w[1 2] })).once + end + + it 'reads a record detail the same way' do + stub_search(contact('1')) + + expect(rows(%w[id], condition_tree: leaf('id', operators::EQUAL, '1'))).to eq([{ 'id' => '1' }]) + end + + # A contact merged into another disappears from the search: the row reads + # as gone rather than as an error, which is what a merge means. + it 'answers no row for a contact that was merged away' do + stub_search + + expect(rows(%w[id], condition_tree: leaf('id', operators::EQUAL, 'merged'))).to be_empty + end + + it 'counts what the ids named' do + stub_search(contact('1')) + + expect(collection.aggregate(nil, filter(condition_tree: leaf('id', operators::EQUAL, '1')), + ForestAdminDatasourceToolkit::Components::Query::Aggregation + .new(operation: 'Count'))) + .to eq([{ 'group' => {}, 'value' => 1 }]) + end + + it 'truncates a set larger than it will read, and says so' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_search(contact('1')) + + rows(%w[id], condition_tree: leaf('id', operators::IN, (1..400).map(&:to_s))) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/asked for 400 records by id/) + expect(WebMock).to have_requested(:post, "#{base}/contacts/search").times(3) + end + end + + describe 'the contacts of an account' do + # `/contacts/search` filters no company field, and `GET + # /companies/{id}/contacts` is what answers the one relation an ops team + # walks the most. Without this route it would be a refusal. + it 'reads them from the company endpoint rather than from the search' do + stub_request(:get, "#{base}/companies/co1/contacts").with(query: hash_including({})) + .to_return(json('type' => 'list', 'data' => [contact('1')], + 'total_count' => 1, 'pages' => {})) + + expect(rows(%w[id], condition_tree: leaf('company_id', operators::EQUAL, 'co1'))) + .to eq([{ 'id' => '1' }]) + expect(WebMock).not_to have_requested(:post, "#{base}/contacts/search") + end + + it 'counts them from the same endpoint' do + stub_request(:get, "#{base}/companies/co1/contacts").with(query: hash_including({})) + .to_return(json('type' => 'list', 'data' => [contact('1')], + 'total_count' => 42, 'pages' => {})) + + expect(collection.aggregate(nil, filter(condition_tree: leaf('company_id', operators::EQUAL, 'co1')), + ForestAdminDatasourceToolkit::Components::Query::Aggregation + .new(operation: 'Count'))) + .to eq([{ 'group' => {}, 'value' => 42 }]) + end + + it 'reports an order this route cannot apply' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_request(:get, "#{base}/companies/co1/contacts").with(query: hash_including({})) + .to_return(json('type' => 'list', 'data' => [contact('1')], + 'total_count' => 1, 'pages' => {})) + + rows(%w[id], condition_tree: leaf('company_id', operators::EQUAL, 'co1'), + sort: sort({ field: 'name', ascending: true })) + + expect(ForestAdminDatasourceIntercom.logger).to have_received(:warn).with(/sort on name/) + end + + # An `and` also carrying a scope names a narrower set than the account + # does, and answering it with the account alone would serve contacts the + # scope excludes. + it 'refuses to take the route for anything but a bare equality' do + expect do + rows(%w[id], condition_tree: branch('And', leaf('company_id', operators::EQUAL, 'co1'), + leaf('role', operators::EQUAL, 'user'))) + end.to raise_error(UnsupportedOperatorError, /cannot filter "company_id"/) + end + end + + describe 'a condition through a relation' do + def stub_admins(*admins) + stub_request(:get, "#{base}/admins").to_return(json('type' => 'admin.list', 'admins' => admins)) + end + + # The owner is a teammate, read whole in one request, and + # `/contacts/search` filters on the key: readable, navigable and + # filterable alike. + it 'resolves the owner against the teammates and filters on the key' do + stub_admins({ 'id' => '493881', 'name' => 'Marie' }) + stub_search(contact('1')) + + rows(%w[id], condition_tree: leaf('owner:name', operators::EQUAL, 'Marie')) + + expect(WebMock).to have_requested(:post, "#{base}/contacts/search") + .with(body: hash_including('query' => { 'field' => 'owner_id', 'operator' => '=', 'value' => '493881' })) + end + + it 'nests the owner under the relation when the projection names it' do + stub_admins({ 'id' => '493881', 'name' => 'Marie' }) + stub_list(contact('1')) + + expect(rows(%w[id owner:name]).first).to eq('id' => '1', + 'owner' => { 'id' => '493881', 'name' => 'Marie' }) + end + + # The endpoint filters no company field, so the relation is there to be + # read and navigated. Refused by name, before the target is read: a + # refusal that spends a request costs exactly what it refuses to do. + it 'refuses a condition through the company, and says what to filter instead' do + expect { rows(%w[id], condition_tree: leaf('company:name', operators::EQUAL, 'Acme')) } + .to raise_error(UnsupportedOperatorError, + %r{resolves to "company_id", on which contacts/search takes no filter}) + expect(WebMock).not_to have_requested(:post, /companies/) + end + end + + describe 'what it will not do' do + it 'refuses to group, Intercom exposing no aggregate endpoint' do + expect do + collection.aggregate(nil, filter, ForestAdminDatasourceToolkit::Components::Query::Aggregation + .new(operation: 'Count', groups: [{ field: 'role' }])) + end.to raise_error(UnsupportedOperatorError, /can only be counted/) + end + + it 'counts what the filter names in one request' do + stub_search(contact('1'), total: 812) + + expect(collection.aggregate(nil, filter(condition_tree: leaf('role', operators::EQUAL, 'user')), + ForestAdminDatasourceToolkit::Components::Query::Aggregation + .new(operation: 'Count'))) + .to eq([{ 'group' => {}, 'value' => 812 }]) + end + end + + describe 'paging' do + it 'asks Intercom for the window the list view named' do + stub_list(contact('1'), contact('2'), contact('3')) + + expect(rows(%w[id], page: page(1, 2)).map { |row| row['id'] }).to eq(%w[2 3]) + end + end + end +end 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 a6cfe8db5..28a6da1c3 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 @@ -120,7 +120,7 @@ def ids(rows) # 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| + %w[tag_names company_name contact_name timeline contact_count].each do |column| expect(collection.fields[column].filter_operators).to be_empty, "#{column} advertises a filter" end end @@ -245,13 +245,14 @@ def ids(rows) .to include('closed_at' => nil, 'reopen_count' => nil) end - # A group conversation has several contacts: the row names how many rather - # than presenting one of them as the one. - it 'carries the contact ids and their count' do + # A group conversation has several contacts: the row names the first, says + # how many there are, and the relation resolves that same first one -- the + # column and the relation cannot disagree. + it 'names the first contact and counts them' do stub_list(conversation('1')) expect(collection.list(nil, filter, nil).first) - .to include('contact_ids' => %w[c1 c2], 'contact_count' => 2) + .to include('contact_id' => 'c1', 'contact_count' => 2) end it 'narrows the row to the projection' do @@ -573,16 +574,20 @@ def aggregation(operation, field: nil, groups: []) 'data' => [{ 'id' => 'c1', 'name' => 'Camille', 'email' => 'camille@acme.test' }])) end - # Denormalized rather than declared as a relation: the Contacts collection - # arrives in lot 4, and a relation whose target is missing is a schema the - # agent refuses to boot on. + # One label on the row plus the relation to navigate, which is the rule + # lot 2.5 set for the ticket labels: `contact_email` is gone, it is a hop + # away on `contact:email`. it 'reads the identity of the page in one request and puts it on the row' do - row = collection.list(nil, filter, %w[id contact_name contact_email]).first + row = collection.list(nil, filter, %w[id contact_name]).first - expect(row).to include('contact_name' => 'Camille', 'contact_email' => 'camille@acme.test') + expect(row).to include('contact_name' => 'Camille') expect(WebMock).to have_requested(:post, "#{base}/contacts/search").once end + it 'no longer publishes the e-mail the relation carries' do + expect(collection.fields.keys).not_to include('contact_email', 'contact_ids') + end + it 'asks for the contacts of the page by id' do collection.list(nil, filter, %w[id contact_name]) 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 3e5530c25..050a16779 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 @@ -514,11 +514,15 @@ def columns # Fifteen conditions per group is Intercom's limit, and a relation reaches # it without trying. Refused by name rather than sent and answered with a # 400 naming neither the limit nor the filter that hit it. + # + # "more than fifteen" rather than a count: the target is read one record + # past what a group holds and no further, so what is known is that it does + # not fit -- see `match_page`. it 'refuses a relation condition matching more records than a group holds' do - stub_admins(*(1..16).map { |index| { 'id' => index.to_s, 'name' => 'Alice' } }) + stub_admins(*(1..20).map { |index| { 'id' => index.to_s, 'name' => 'Alice' } }) expect { rows(%w[id], condition_tree: leaf('admin_assignee:name', operators::EQUAL, 'Alice')) } - .to raise_error(UnsupportedOperatorError, /names 16 records.*15 conditions per group/m) + .to raise_error(UnsupportedOperatorError, /names more than 15 records.*15 conditions per group/m) end # `/tickets/search` filters no state id -- the table carries none -- so the diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/datasource_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/datasource_spec.rb index 91ad2f84f..0dc29ba1d 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/datasource_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/datasource_spec.rb @@ -7,24 +7,28 @@ module ForestAdminDatasourceIntercom end # The reference collections come first: they are what turns an assignee id - # into a teammate and a state id into a label. Conversations follow, Tickets - # next. The membership sits with the two collections it joins: a many-to-many + # into a teammate and a state id into a label. Contacts and Companies + # follow, before the two collections whose relations point at them. + # The membership sits with the two collections it joins: a many-to-many # needs a collection to travel through, and Intercom exposes none. it 'publishes the collections of the lot' do expect(datasource.collections.keys) .to eq(%w[IntercomAdmin IntercomTeam IntercomTeamMembership IntercomTicketType IntercomTicketState - IntercomConversation IntercomTicket]) + IntercomContact IntercomCompany IntercomConversation IntercomTicket]) end - # The one read a boot performs: the attributes a workspace declares on its - # ticket types are columns of the Tickets collection, and a ticket payload - # carries the values of its own type only, so they cannot be discovered from - # the records. - it 'introspects the ticket-type attributes while registering, and reads nothing else' do + # The three reads a boot performs, and no fourth: the attributes a workspace + # declares on its ticket types, on its contacts and on its companies are + # columns of those collections, and a payload carries the values of the + # attributes that record happens to have been given, never their + # definitions. + it 'introspects the workspace attributes while registering, and reads nothing else' do datasource expect(WebMock).to have_requested(:get, /ticket_types/).once - expect(WebMock).not_to have_requested(:get, /conversations|admins|teams/) + expect(WebMock).to have_requested(:get, /data_attributes/).with(query: { 'model' => 'contact' }).once + expect(WebMock).to have_requested(:get, /data_attributes/).with(query: { 'model' => 'company' }).once + expect(WebMock).not_to have_requested(:get, %r{conversations|admins|teams|companies/list}) end # A token without that permission costs the attribute columns, never the @@ -37,8 +41,22 @@ module ForestAdminDatasourceIntercom expect(datasource.get_collection('IntercomTicket').fields.keys).not_to include('_default_title_') end + # The same guarantee on the other two models, and it is what the acceptance + # criterion of lot 4 asks for: a token missing a permission costs the + # columns it could not read, and the collection still boots. + it 'boots the contact and company collections when their introspection is refused' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_request(:get, /data_attributes/).to_return(status: 403, body: '{}', + headers: { 'Content-Type' => 'application/json' }) + + expect(datasource.get_collection('IntercomContact').fields.keys).to include('email') + expect(datasource.get_collection('IntercomCompany').fields.keys).to include('name') + end + it 'configures a client from the options it is handed' do stub_ticket_types(base: 'https://api.eu.intercom.io') + stub_data_attributes('contact', base: 'https://api.eu.intercom.io') + stub_data_attributes('company', base: 'https://api.eu.intercom.io') configured = described_class.new(access_token: 's3cr3t', region: :eu, rate_limiter: nil) expect(configured.configuration.url).to eq('https://api.eu.intercom.io') 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 b203c5545..9b36bf607 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 @@ -15,8 +15,27 @@ def field_row(overrides = {}) # 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]) + it 'reads the three search endpoints' do + expect(described_class.endpoints).to eq(%w[conversations tickets contacts]) + end + + # The asymmetry the per-endpoint table exists for, asserted on the file + # that ships rather than on a table a spec built: a date is bounded on + # every endpoint, and only two of them take the closed bounds and the + # inequality. + it 'carries the measured date restrictions of /contacts/search' do + expect(described_class.fetch('contacts').field('created_at').operators).to eq(['>', '<']) + expect(described_class.fetch('conversations').field('created_at').operators) + .to include('>=', '<=', '!=') + end + + # Intercom sorts one endpoint and ignores the sort it is sent on the other + # two, so a sortable column outside contacts would promise an order that + # never happens. + it 'declares a sortable column on the one endpoint that sorts' do + expect(described_class.fetch('contacts').sortable_columns).not_to be_empty + expect(described_class.fetch('conversations').sortable_columns).to be_empty + expect(described_class.fetch('tickets').sortable_columns).to be_empty end it 'names the path each endpoint is searched through' do @@ -85,7 +104,8 @@ def field_row(overrides = {}) end it 'lists exactly the columns each endpoint filters' do - { 'IntercomConversation' => 'conversations', 'IntercomTicket' => 'tickets' }.each do |collection, endpoint| + { 'IntercomConversation' => 'conversations', 'IntercomTicket' => 'tickets', + 'IntercomContact' => 'contacts' }.each do |collection, endpoint| row = filterable.lines.find { |line| line.start_with?("| `#{collection}` |") } listed = row.to_s.scan(/`([a-z_]+)`/).flatten @@ -111,6 +131,14 @@ def field_row(overrides = {}) .to raise_error(ConfigurationError, /belongs in the refused table/) end + # YAML reads an unquoted `true` as a boolean and a quoted one as a string, + # which is truthy in Ruby: a typo would publish a sortable column the + # endpoint never sorts. + it 'refuses a sortable flag that is not a boolean' do + expect { table(fields: { 'created_at' => field_row('sortable' => 'true') }) } + .to raise_error(ConfigurationError, /sortable "true" is neither true nor false/) + 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/) @@ -140,8 +168,8 @@ def field_row(overrides = {}) 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"/) + expect { described_class.fetch('companies') } + .to raise_error(ConfigurationError, /Unknown Intercom search endpoint "companies"/) end it 'is measured once the probe has stamped a date on it' do diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/schema/data_attributes_introspector_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/schema/data_attributes_introspector_spec.rb new file mode 100644 index 000000000..6896ccf33 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/schema/data_attributes_introspector_spec.rb @@ -0,0 +1,136 @@ +module ForestAdminDatasourceIntercom + module Schema + RSpec.describe DataAttributesIntrospector do + subject(:introspector) { described_class.new(Client.new(configuration), model: 'contact') } + + let(:configuration) { Configuration.new(access_token: 's3cr3t', rate_limiter: nil) } + let(:base) { configuration.url } + + def json(payload, status = 200) + { status: status, body: payload.to_json, headers: { 'Content-Type' => 'application/json' } } + end + + def attribute(name, overrides = {}) + { 'type' => 'data_attribute', 'model' => 'contact', 'name' => name, 'label' => name, + 'data_type' => 'string', 'custom' => true, 'archived' => false, + 'api_writable' => true }.merge(overrides) + end + + def stub_attributes(*attributes, model: 'contact') + stub_request(:get, "#{base}/data_attributes").with(query: { 'model' => model }) + .to_return(json('type' => 'list', 'data' => attributes)) + end + + it 'reads the attributes of the model it was built for' do + stub_attributes(attribute('paid_subscriber'), model: 'company') + + expect(described_class.new(Client.new(configuration), model: 'company').attributes.map(&:name)) + .to eq(['paid_subscriber']) + end + + # The standard attributes are columns the collection declares by hand, + # with the filters the search table measured. Publishing them again under + # their `custom_attributes` name would show one fact twice, the + # unfilterable copy winning nothing. + it 'leaves out the attributes Intercom defines itself' do + stub_attributes(attribute('paid_subscriber'), attribute('email', 'custom' => false)) + + expect(introspector.attributes.map(&:name)).to eq(['paid_subscriber']) + end + + it 'leaves out an archived attribute, which the workspace stopped offering' do + stub_attributes(attribute('paid_subscriber'), attribute('old_plan', 'archived' => true)) + + expect(introspector.attributes.map(&:name)).to eq(['paid_subscriber']) + end + + it 'leaves out an attribute with no name to be a column of' do + stub_attributes(attribute(''), 'not a hash') + + expect(introspector.attributes).to be_empty + end + + # Read and carried although every column of this lot is published + # read-only: it is what lot 4b needs to tell an attribute it may write + # from one Intercom fills in itself, and reading it again then would be a + # second boot-time round trip. + it 'carries api_writable for the lot that writes' do + stub_attributes(attribute('paid_subscriber'), attribute('lifetime_value', 'api_writable' => false)) + + expect(introspector.attributes.map { |a| [a.name, a.api_writable] }) + .to eq([['paid_subscriber', true], ['lifetime_value', false]]) + end + + it 'maps each Intercom data type onto what Forest renders' do + stub_attributes(attribute('a', 'data_type' => 'integer'), attribute('b', 'data_type' => 'float'), + attribute('c', 'data_type' => 'boolean'), attribute('d', 'data_type' => 'date')) + + expect(introspector.attributes.map(&:column_type)).to eq(%w[Number Number Boolean Date]) + end + + # Showing the value Intercom sent beats hiding a column because its type + # is new. + it 'reads an unknown data type as a string rather than dropping the column' do + stub_attributes(attribute('paid_subscriber', 'data_type' => 'quantum')) + + expect(introspector.attributes.first.column_type).to eq('String') + end + + # Forest lists the fields of a request in a comma-separated query + # parameter and names a field through a relation with a colon: either one + # in a column name breaks the projection before the page is read. + it 'takes the commas and colons out of a column name, keeping the name the payload uses' do + stub_attributes(attribute('Plan, tier: current')) + + expect(introspector.attributes.map { |a| [a.name, a.column_name] }) + .to eq([['Plan, tier: current', 'Plan tier current']]) + end + + it 'unescapes a name Intercom handed back escaped' do + stub_attributes(attribute('Ce que j'ai vérifié')) + + expect(introspector.attributes.first.column_name).to eq("Ce que j'ai vérifié") + end + + # Two attributes landing on one column would share an entry, and the + # second's values would be read under the first's name -- wrong values + # rather than missing ones. + it 'leaves out an attribute colliding with one already kept, and says which' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_attributes(attribute('Plan, tier'), attribute('Plan: tier')) + + expect(introspector.attributes.map(&:name)).to eq(['Plan, tier']) + expect(ForestAdminDatasourceIntercom.logger) + .to have_received(:warn).with(/"Plan: tier" is left out.*"Plan tier"/) + end + + it 'keeps an attribute whose name is nothing but separators out of the schema' do + stub_attributes(attribute(',,')) + + expect(introspector.attributes).to be_empty + end + + # A token without the permission costs the columns, never the boot. + it 'answers no attribute when Intercom refuses the read, and says so' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_request(:get, "#{base}/data_attributes").with(query: { 'model' => 'contact' }) + .to_return(json( + { 'type' => 'error.list', + 'errors' => [{ 'code' => 'forbidden' }] }, 403 + )) + + expect(introspector.attributes).to eq([]) + expect(ForestAdminDatasourceIntercom.logger) + .to have_received(:warn).with(/could not read the contact attributes \(HTTP 403\)/) + end + + it 'reads Intercom once, however many times it is asked' do + stub_attributes(attribute('paid_subscriber')) + + 2.times { introspector.attributes } + + expect(WebMock).to have_requested(:get, /data_attributes/).once + 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 405d2f85f..105c4eac7 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 @@ -151,7 +151,7 @@ def run_probe(field, type) end it 'refuses an endpoint the table does not declare' do - expect { described_class.call(['--endpoint', 'contacts', '--token', 's3cr3t']) } + expect { described_class.call(['--endpoint', 'companies', '--token', 's3cr3t']) } .to raise_error(ConfigurationError, /Unknown Intercom search endpoint/) end @@ -160,6 +160,8 @@ def run_probe(field, type) stub_search(code: 'invalid_field') stub_request(:post, "#{base}/conversations/search") .to_return(json({ 'type' => 'error.list', 'errors' => [{ 'code' => 'invalid_field' }] }, 400)) + stub_request(:post, "#{base}/contacts/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') diff --git a/packages/forest_admin_datasource_intercom/spec/spec_helper.rb b/packages/forest_admin_datasource_intercom/spec/spec_helper.rb index 056ee440e..4cbaea580 100644 --- a/packages/forest_admin_datasource_intercom/spec/spec_helper.rb +++ b/packages/forest_admin_datasource_intercom/spec/spec_helper.rb @@ -27,16 +27,25 @@ # and a fixture is read by everyone who clones the repo. WebMock.disable_net_connect!(allow_localhost: true) -# A datasource introspects the ticket-type attributes while it registers its -# collections, so every spec building one issues that read. The base url is not -# taken from the datasource on purpose: reading it would build the datasource, -# and boot the very read this stubs. +# A datasource introspects the ticket-type attributes and the contact and +# company attributes while it registers its collections, so every spec building +# one issues those three reads. The base url is not taken from the datasource on +# purpose: reading it would build the datasource, and boot the very reads this +# stubs. module IntercomBootStubs def stub_ticket_types(*types, base: ForestAdminDatasourceIntercom::Configuration::REGION_HOSTS[:us]) stub_request(:get, "#{base}/ticket_types") .to_return(status: 200, body: { 'type' => 'list', 'data' => types }.to_json, headers: { 'Content-Type' => 'application/json' }) end + + def stub_data_attributes(model, *attributes, + base: ForestAdminDatasourceIntercom::Configuration::REGION_HOSTS[:us]) + stub_request(:get, "#{base}/data_attributes").with(query: { 'model' => model }) + .to_return(status: 200, + body: { 'type' => 'list', 'data' => attributes }.to_json, + headers: { 'Content-Type' => 'application/json' }) + end end RSpec.configure do |config| @@ -55,5 +64,7 @@ def stub_ticket_types(*types, base: ForestAdminDatasourceIntercom::Configuration config.before do WebMock.reset! stub_ticket_types + stub_data_attributes('contact') + stub_data_attributes('company') end end From 642c2ff76588b15292b08b62b73b5a57d9a62838 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Mon, 7 Sep 2026 16:52:07 +0200 Subject: [PATCH 2/5] docs(intercom): the third tier, the lookups and the 360 degrees What an operator needs before meeting any of it in production: the offset tier and why companies escape the cursor walker, the four lookups and the two of them that name a column, what a merge does to a row, which relation traversals are refused and which are a spec row the probe has yet to confirm. Corrects two statements lot 4 makes partly false: there is offset pagination, on companies alone, and one endpoint does sort. Co-Authored-By: Claude Opus 5 (1M context) --- .../README.md | 191 +++++++++++++++--- 1 file changed, 161 insertions(+), 30 deletions(-) diff --git a/packages/forest_admin_datasource_intercom/README.md b/packages/forest_admin_datasource_intercom/README.md index c44ed9995..3a3e431af 100644 --- a/packages/forest_admin_datasource_intercom/README.md +++ b/packages/forest_admin_datasource_intercom/README.md @@ -59,10 +59,11 @@ beats not running. ### Token permissions A read-only token is enough, and is what to recommend for this lot. A permission the token lacks -costs **columns or a collection, never the boot of the agent**: the ticket-type introspection -degrades to no attribute column, a collection whose endpoint answers 403 fails its own page, and a -token that cannot read `/admins` or `/teams` leaves the `admin_names` / `team_names` column empty -rather than failing the page it is on. +costs **columns or a collection, never the boot of the agent**: the three boot-time introspections +each degrade to no attribute column, a collection whose endpoint answers 403 fails its own page, and +a token that cannot read `/admins` or `/teams` leaves the `admin_names` / `team_names` column empty +rather than failing the page it is on. A token denied contacts or companies costs those two +collections and the `contact_name` column, and leaves everything else standing. A **relation is the exception**, and it is worth knowing before scoping a token: resolving one reads the target endpoint, and that read is not guarded the way the names above are. A token denied @@ -81,19 +82,29 @@ denied. Scope the token to the endpoints in the table below, or to none of them. | `IntercomTeamMembership` | `GET /teams` | read whole | yes, exactly | | `IntercomTicketType` | `GET /ticket_types` | read whole | yes, exactly | | `IntercomTicketState` | `GET /ticket_states` | read whole | yes, exactly | +| `IntercomContact` | `GET /contacts`, `POST /contacts/search`, `GET /companies/{id}/contacts` | cursor | yes, exactly | +| `IntercomCompany` | `POST /companies/list`, `GET /companies?...`, `GET /companies/{id}` | **offset** | yes, exactly | -Two tiers, and they behave differently on purpose. +Three tiers, and they behave differently on purpose. **Read whole** — admins, teams, team memberships, ticket types, ticket states. Their endpoints answer in one response, so filtering, sorting, paging and counting them in memory is *exact*: the records in hand are every record Intercom holds. These are the only collections that can be filtered, sorted and grouped in this lot, and the only ones a chart may group by. The cost is bandwidth, not correctness. -**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 translated into -Intercom's search DSL and walked through the search endpoint. What the translation cannot express is -**refused by name** — see [Filtering](#filtering). +**Cursor** — conversations, tickets and contacts. 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 translated +into Intercom's search DSL and walked through the search endpoint. What the translation cannot +express is **refused by name** — see [Filtering](#filtering). Contacts add two routes of their own, +both described under [Contacts](#contacts). + +**Offset** — companies, and nothing else. `POST /companies/list` takes a **page number**, which is +what a list view asks for: page 7 is one request rather than six pages walked to reach it, with no +cap and no truncation warning. It is the one place the [first limitation +below](#what-the-api-cannot-do-and-what-this-does-about-it) does not apply. What it pays for that is +filtering — there is no company search endpoint at all, so what a filter may say is a handful of +exact lookups and nothing else. See [Companies](#companies). ## Relations @@ -120,6 +131,12 @@ sized for reference collections, which is what every target here is. | `IntercomTeam` | `admins` | `IntercomAdmin` | no (many-to-many) | | `IntercomAdmin` | `teams` | `IntercomTeam` | no (many-to-many) | | `IntercomTeamMembership` | `team`, `admin` | `IntercomTeam`, `IntercomAdmin` | yes | +| `IntercomConversation` | `contact` | `IntercomContact` | yes | +| `IntercomTicket` | `contact` | `IntercomContact` | **spec, unprobed** — see below | +| `IntercomContact` | `owner` | `IntercomAdmin` | yes | +| `IntercomContact` | `company` | `IntercomCompany` | **no** — read and navigate only | +| `IntercomContact` | `conversations`, `tickets` | `IntercomConversation`, `IntercomTicket` | no (one-to-many) | +| `IntercomCompany` | `contacts` | `IntercomContact` | no (one-to-many) | Every one of them is **read-only**: this lot writes nothing, and Intercom exposes no endpoint that writes a team membership at all. @@ -143,6 +160,27 @@ lots published: one readable form plus a relation to navigate, rather than two w They are read only when a projection asks for them, and a token that cannot read the other side costs the column and nothing else — never the page, and never the relation. +**The 360 degrees is those last four rows.** From a ticket or a conversation, `contact` reaches the +person who wrote in; from them, `conversations` and `tickets` list everything they ever opened, and +`company` reaches their account, whose `contacts` lists their colleagues. Each of those lists is one +request: `/conversations/search` matches a conversation against one of its contact ids, and +`GET /companies/{id}/contacts` answers the contacts of an account — which is the one relation +`/contacts/search` could not have resolved, filtering no company field. + +**A conversation has several contacts, and the relation names the first of them** — the same one +`contact_name` and `contact_count` describe, so the column and the relation cannot disagree. The +others are a hop away: open that contact and read their conversations. The alternative, a +many-to-many through a join collection, would have been the honest cardinality at the price of three +collections of plumbing in the interface; naming the first contact and counting them is what lot 1 +already published, and lot 4 promotes it rather than replacing it. + +Two of these carry a caveat worth reading before scoping a token or writing a segment. The **ticket +side is a `spec` row the probe has not confirmed**: whether `/tickets/search` filters on +`contact_ids` at all is unmeasured, and if it does not, the relation stays navigable and the filter +moves to the refusal table — exactly what happened to the ticket `state`. And **the company +traversal is refused by name**: `/contacts/search` filters no company field, so `company:name` is +answered with a message saying to filter from the company side instead. + The same rule settled the ticket labels: `state_label` and `ticket_type_name` stay on the row, `state_category` and `state_external_label` are gone — they are a hop away, on the `state` relation, and neither was ever filterable, so no segment, scope or saved filter could rest on them. @@ -161,18 +199,21 @@ Where Forest asks for something Intercom has no equivalent for, this datasource message naming the reason** rather than answering something that looks right and is not. Those arrive as a 400 carrying the text. -- **No offset pagination.** Intercom hands out the page after a cursor and documents that jumping to - page N is unsupported, so reaching page 20 costs 20 sequential requests. The walk is capped at 50 - pages / 7 500 records and every truncation is logged, naming the window it stopped in. +- **No offset pagination, except on companies.** Intercom hands out the page after a cursor and + documents that jumping to page N is unsupported, so reaching page 20 costs 20 sequential requests. + The walk is capped at 50 pages / 7 500 records and every truncation is logged, naming the window + it stopped in. `POST /companies/list` is the exception and takes a page number, which is why + companies escape the walker and its caps entirely. - **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. +- **One endpoint sorts, and it is `/contacts/search`.** Everything else comes back in the order the + API imposes: `POST /companies/list` has no order parameter at all, and the other two search + endpoints **accept a `sort` and ignore it** — measured, it raises nothing and changes nothing. + Since that is undetectable at runtime, no column of `IntercomConversation`, `IntercomTicket` or + `IntercomCompany` is declared sortable and a requested order is reported in the log. The + collections read whole sort in memory, exactly, and Contacts sort server-side on the columns the + measured table declares — see [Contacts](#contacts). - **No aggregate endpoint.** Counting is free and exact — `total_count` counts what the query names, not what a page held — so the record counter is one request. Anything beyond a count is refused on the cursor collections: grouping over the pages a walk collected would look exact while answering @@ -267,6 +308,19 @@ refuses a search by name. ### What is not filterable, and why +- **every column of a company but two.** There is no `/companies/search`: Intercom looks a company + up by `name`, by `company_id`, by `tag_id` or by `segment_id`, one exact value at a time, and the + first two are the ones that name a column of the collection. Everything else — the industry, the + plan, the monthly spend — is refused by name. Filtering by tag or by segment belongs with the lot + that adds those collections; +- **a contact's `company_id`, `company_count`, `avatar` and `session_count`** — the endpoint filters + none of them. Reach the contacts of an account from the account instead, through its `contacts` + relation, which is one request; +- **the custom attributes of a contact or a company.** They are filtered as + `custom_attributes.{name}`, by name — the ambiguity that keeps ticket attributes display-only does + not arise here — but which operators Intercom answers on each data type has not been measured, and + this package publishes no filter it has not seen work. They ship typed and display-only, and the + probe is what turns that around; - **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; @@ -294,8 +348,14 @@ What Intercom is really filtered on is the foreign key: the **target says which match**, over every record it holds rather than over a page, and the ids it names become the condition the search carries. -That is exact, and it has three visible edges: +That is exact, and it has four visible edges: +- **The target is read one record past what a group may hold, and no further.** Against a collection + read whole that costs nothing — every record is in hand — but Contacts are a page of something + far larger, and resolving `contact:email contains "@"` over a whole workspace to then refuse the + fan-out it comes to would spend a full cursor walk on a filter that was never going to be + answered. So the read is bounded, and the refusal says "more than fifteen" rather than a count it + deliberately did not go and measure. - Intercom takes no membership operator on these fields, so several matches become **one equality per match**, inside an `OR` — which counts against the fifteen conditions a group allows. A relation condition matching more records than that is refused by name rather than sent and answered with a @@ -306,10 +366,10 @@ That is exact, and it has three visible edges: being the scarcer of the two. - A condition the target matched **no record** with names no row, and the DSL cannot say so: the search is skipped entirely rather than sent as a filter that would come back with everything. -- A relation whose foreign key the endpoint does not filter — the ticket `state` — is refused with a - message saying which of the two it is: the relation is there to be read and navigated. Whether - `/tickets/search` filters a state id at all is one of the probe's open questions; the answer lands - in the table, not in an assumption. +- A relation whose foreign key the endpoint does not filter — the ticket `state`, a contact's + `company` — is refused with a message saying which of the two it is: the relation is there to be + read and navigated. Whether `/tickets/search` filters a state id or a contact id at all is one of + the probe's open questions; the answer lands in the table, not in an assumption. On the collections read whole the same condition costs nothing: they filter in memory, so the ids go in as a plain membership and none of the DSL's limits apply. @@ -389,6 +449,64 @@ schema that changes shape whenever the customer adds a type. Until that trade is 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. +## Contacts + +The people who write in, users and leads alike. Cursor-paginated like conversations and tickets, +with two routes of its own and one thing no other collection has. + +**Intercom sorts this one.** `POST /contacts/search` is the only endpoint of the whole API that +takes a `sort` and applies it, so these are the only sortable columns of the datasource: `name`, +`email`, `created_at`, `updated_at`, `signed_up_at`, `last_seen_at`, `last_contacted_at`, +`last_replied_at`. The set is deliberately narrower than the documentation implies — nothing has +been measured, and a sort Intercom refuses is a list view that fails rather than one that comes back +unordered. A sort on any other column, or on two columns at once, is reported in the log and the +rows come back in the API's order: Intercom takes a single `{ field, order }`, and honouring the +first clause of two would order the page by something nobody asked for. + +An order is also what routes a plain list view through the search endpoint, the listing sorting +nothing: the read then carries the predicate matching everything that Tickets already send. + +**Its date operators are narrower than the other two endpoints'** — measured, 25 August 2026: +`/contacts/search` refuses `>=`, `<=` and `!=` on a date where `/conversations/search` and +`/tickets/search` take them. Nothing is lost that an operator can see, a Date column publishing the +two bounds alone everywhere in this datasource, but it is why the operator table is per endpoint. + +**A set of ids is read in one request** — `id IN [...]`, which this endpoint answers and no other +does — a hundred at a time, rather than one request per record. It is what makes a related list of +contacts affordable. + +**`company_id equals X` reads `GET /companies/{id}/contacts`.** The search filters no company field, +so without that route the contacts of an account would be a refusal rather than a list. It is a bare +equality only: an `and` also carrying a permission scope names a narrower set than the account does, +and answering it with the account alone would serve contacts the scope excludes. + +**A merged contact reads as gone, not as an error.** Intercom drops it from the listing and from the +search, and the record lives on under the id it was merged into. A row pointing at the old id comes +back empty rather than failing the page. + +The custom attributes a workspace declares on its contacts are introspected once at boot from +`GET /data_attributes?model=contact`, typed from `data_type`, and published display-only. + +## Companies + +The accounts contacts belong to, and the collection that behaves least like the others. + +**Paginated by offset**, which is the tier above. **Looked up, not searched**: `name` and +`company_id` — the identifier the customer's own system gave the account, not Intercom's — are the +two filters it publishes, each an exact equality, and anything else is refused by name. A record is +read through `GET /companies/{id}`, and a set of ids one request each, capped at 25 with the +truncation logged. + +`GET /companies/scroll` exists and is **deliberately rejected**: one open scroll per application, +expiring after a minute, cannot serve two operators looking at a list at the same time. + +A contact carries its accounts as a list of ids and nothing else, so **projecting `company:name` on +a contact list costs one request per distinct account on the page**. Reading the account from the +contact's record page, or listing contacts from the account, both cost one. + +Custom attributes are introspected at boot the same way, from `GET /data_attributes?model=company`, +and published display-only for the same reason. + ## Rate limits Intercom meters the app and, above it, the whole workspace — 25 000 requests a minute shared with @@ -422,10 +540,19 @@ The body of a conversation is raw personal data, and this datasource is built on ## Boot-time introspection -Constructing the datasource performs exactly **one** read: `GET /ticket_types`, for the attribute -columns of `IntercomTicket`. It runs on the boot connection — short timeouts, one quick retry — so a -slow Intercom cannot turn a Rails boot into minutes the operator sits through, and it degrades to no -attribute column rather than to a failed boot. +Constructing the datasource performs exactly **three** reads, and they are all of the same kind: +`GET /ticket_types` for the attribute columns of `IntercomTicket`, and +`GET /data_attributes?model=contact` and `?model=company` for those of `IntercomContact` and +`IntercomCompany`. A payload carries the values of the attributes that record happens to have been +given, never their definitions, which is why they cannot be discovered from the records. + +All three run on the boot connection — short timeouts, one quick retry — so a slow Intercom cannot +turn a Rails boot into minutes the operator sits through, and each degrades to no attribute column +rather than to a failed boot. + +`api_writable` is read alongside each attribute and kept, although every column of this lot is +published read-only: it is what tells an attribute the API may write from one Intercom fills in +itself, and reading it again later would be a second boot-time round trip. Everything else is read when a collection is listed, so an agent boots whatever Intercom is doing. @@ -433,11 +560,15 @@ Everything else is read when a collection is listed, so an agent boots whatever | Lot | What it brings | | --- | --- | -| 3 | Writes and business actions: reply, close, snooze, reopen, assign, tag, convert | -| 4 | Contacts and companies, and the relations towards them promoted from today's denormalized columns | +| 3 | Writes and business actions on tickets and conversations: reply, close, snooze, reopen, assign, tag, convert | +| 4b | Writes on contacts and companies: create, update, archive, block, merge, attach and detach | | 5 | Notes, tags, segments | | 6 | Bounded group-by and the reporting export | +Two questions this lot leaves in the table rather than in an assumption, both for +`bin/probe_search_fields` to answer against the customer's workspace: whether `/tickets/search` +filters on `contact_ids`, and which operators `/contacts/search` answers on a custom attribute. + ## Development ```bash From 6667c23a6a530296cccf04d7ba40d17bf137db6e Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Mon, 7 Sep 2026 16:53:56 +0200 Subject: [PATCH 3/5] test(intercom): the listing behind a contact's conversations A one-to-many is served by the agent listing the target on the key, so what the relation really rests on is the endpoint filtering a contact id -- measured on conversations, a spec row on tickets. Co-Authored-By: Claude Opus 5 (1M context) --- .../collections/conversation_spec.rb | 27 +++++++++++++++++++ .../collections/ticket_spec.rb | 17 ++++++++++++ 2 files changed, 44 insertions(+) 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 28a6da1c3..754df26d9 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 @@ -199,6 +199,33 @@ def ids(rows) end end + # The other half of the 360 degrees: `IntercomContact#conversations` is a + # one-to-many, and the agent serves it by listing this collection on the key + # -- so what makes that list possible is this endpoint filtering on a + # contact id at all. The column is singular and the wire field plural: the + # row names its first contact, the endpoint asks whether a contact is one of + # the conversation's. + describe 'the conversations of a contact' do + it 'filters on the contact id the relation resolves to' do + stub_search(conversation('1')) + + collection.list(nil, filter(condition_tree: leaf('contact_id', operators::EQUAL, 'c1')), %w[id]) + + expect(WebMock).to have_requested(:post, "#{base}/conversations/search") + .with(query: hash_including({}), + body: hash_including('query' => { 'field' => 'contact_ids', 'operator' => '=', 'value' => 'c1' })) + end + + it 'counts them in one request' do + stub_search(conversation('1'), total: 37) + + expect(collection.aggregate(nil, filter(condition_tree: leaf('contact_id', operators::EQUAL, 'c1')), + ForestAdminDatasourceToolkit::Components::Query::Aggregation + .new(operation: 'Count'))) + .to eq([{ 'group' => {}, 'value' => 37 }]) + end + end + describe '#list' do it 'reads the listing endpoint as plain text and pages by cursor' do stub_list(conversation('1')) 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 050a16779..45c9c52e4 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 @@ -380,6 +380,23 @@ def columns # exists. What Intercom is really filtered on is the foreign key: the target # says which of its records match -- over every record it holds, not over a # page -- and the ids it names are what the search carries. + # `IntercomContact#tickets` is a one-to-many the agent serves by listing this + # collection on the key. The row it rests on is `spec`, not `measured`: + # whether `/tickets/search` filters on a contact id at all is one of the + # probe's questions, and the answer moves the row into the table or into the + # refusals. + describe 'the tickets of a contact' do + it 'filters on the contact id the relation resolves to' do + stub_search(ticket('1')) + + collection.list(nil, filter(condition_tree: leaf('contact_id', operators::EQUAL, 'c1')), %w[id]) + + expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + .with(body: hash_including('query' => { 'field' => 'contact_ids', 'operator' => '=', + 'value' => 'c1' })) + end + end + describe 'a condition through a relation' do it 'filters on the foreign key the target resolved to' do stub_admins('id' => '493881', 'name' => 'Alice') From 16695cde2e26f33519a974b77fdbff3b589bea7b Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Mon, 7 Sep 2026 17:14:08 +0200 Subject: [PATCH 4/5] feat(intercom): publish the thread of a ticket Lot 1 paid for the parts of every ticket and published five columns derived from them, never the exchange itself. It costs no request: Intercom returns the parts in the search response and offers no way to ask it not to, so the page pays for them whatever the projection says. The builder moves out of Conversation into a shared module, with two hooks for what the two resources do differently: where their parts live, and whether anything opens the thread before them. A ticket has no source object, so its thread starts on its first part. An empty list means an empty thread here, where a conversation read from a listing carries no parts at all and keeps a nil that reads as unknown. Ticket reads now carry display_as=plaintext, which lot 1 sent on conversations alone: the bodies are HTML written by end customers, and there was no body on a ticket row until now. Co-Authored-By: Claude Opus 5 (1M context) --- .../README.md | 18 ++++- .../collections/conversation.rb | 4 +- .../collections/conversation/timeline.rb | 45 ++--------- .../collections/ticket.rb | 26 ++++++ .../collections/timeline.rb | 53 ++++++++++++ .../query/search_fields.yml | 6 ++ .../collections/ticket_spec.rb | 80 ++++++++++++++++--- 7 files changed, 177 insertions(+), 55 deletions(-) create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/timeline.rb diff --git a/packages/forest_admin_datasource_intercom/README.md b/packages/forest_admin_datasource_intercom/README.md index 3a3e431af..7ca2838d0 100644 --- a/packages/forest_admin_datasource_intercom/README.md +++ b/packages/forest_admin_datasource_intercom/README.md @@ -410,8 +410,13 @@ Intercom returns the parts **only when retrieving a single conversation**, so: A conversation is capped at its **500 most recent parts**; a very long thread is therefore partial, and says so nowhere but here. -Contact name and e-mail are denormalized onto the row by **one bulk read per page**, not one per -row, and only when the projection names them. A failure there costs those two columns, not the page. +**The internal notes of the team are in the thread**, next to what the customer was told. That is +what a thread is on Intercom, and publishing half of it would be the more surprising answer — but it +is worth knowing before opening the collection to a role that should not read them. + +The contact's name is denormalized onto the row by **one bulk read per page**, not one per row, and +only when the projection names it. A failure there costs that column, not the page. The e-mail is a +hop away, on the `contact` relation. ## Tickets @@ -439,6 +444,15 @@ Four things to know about them: Both are **display only**, and not temporarily: `/tickets/search` filters on neither and ignores a sort, so neither advertises an operator. +**The thread is published too, and it is free here.** The same `timeline` column a conversation +carries — who said what, when, and through which kind of event, internal notes and state changes +included — built from the parts the response already holds. No request per row and no cap: where a +conversation read from a listing carries no parts at all and leaves the column `nil` for *unknown*, +a ticket always carries them, so an empty list means an empty thread. Both reads ask Intercom for +`display_as=plaintext`: the bodies are HTML written by end customers, and rendering third-party +markup inside Forest is neither safe nor useful. The 500-part ceiling applies here as well, which is +the same truncation that can hide a closure date. + 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 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 43cfedb48..09c8b390a 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 @@ -10,9 +10,11 @@ module Collections # customers, and rendering third-party HTML inside Forest is neither safe nor # useful (R10). # Long by line count only: most of it declares the columns, one call each. - class Conversation < CursorCollection + class Conversation < CursorCollection # rubocop:disable Metrics/ClassLength include ContactIdentity include Conversation::Serializer + # The shared thread, then the hooks this collection puts over it. + include Collections::Timeline include Conversation::Timeline # How many conversations of one page may have their timeline read. The diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation/timeline.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation/timeline.rb index 9b757f6c8..bb7fe62cd 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation/timeline.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/conversation/timeline.rb @@ -1,18 +1,12 @@ module ForestAdminDatasourceIntercom module Collections class Conversation < CursorCollection - # The thread of a conversation, as a structured list the record view can - # render: who said what, when, and through which kind of event. + # What a conversation adds to the shared thread: where its parts live, and + # the entry that opens it. # - # Two things this exists to get right. The opening message lives in - # `source`, not in the parts -- a timeline built from the parts alone opens - # on the first reply and loses what the customer actually asked. And - # `part_type` is kept on every entry: an assignment, a note and a reply are - # not the same event, and a thread that flattens them reads as a - # conversation that never happened the way it did. - # - # Intercom caps a conversation at its 500 most recent parts; the entry - # count is therefore what is in hand, not necessarily what exists. + # The opening message lives in `source`, not in the parts -- a timeline + # built from the parts alone opens on the first reply and loses what the + # customer actually asked. module Timeline # The pseudo type of the opening entry. Not an Intercom part type: it is # the source, and calling it `comment` would make it indistinguishable @@ -21,13 +15,6 @@ module Timeline private - def build_timeline(conversation) - attrs = conversation.is_a?(Hash) ? conversation : {} - entries = [source_entry(attrs)].compact - - entries + (parts_of(attrs) || []).map { |part| part_entry(part) } - end - # nil rather than an empty list when the payload carries no parts at all: # a listing response has none, and reading that as "this conversation is # empty" is exactly the answer that looks complete without being it. @@ -39,7 +26,7 @@ def parts_of(conversation) parts.is_a?(Array) ? parts : nil end - def source_entry(attrs) + def opening_entry(attrs) source = attrs['source'] return nil unless source.is_a?(Hash) @@ -47,26 +34,6 @@ def source_entry(attrs) body: source['body'], attachments: source['attachments']) .merge('id' => stringify_id(source['id'])) end - - def part_entry(part) - attrs = part.is_a?(Hash) ? part : {} - - entry(part_type: attrs['part_type'], created_at: attrs['created_at'], author: attrs['author'], - body: attrs['body'], attachments: attrs['attachments']) - .merge('id' => stringify_id(attrs['id']), 'redacted' => attrs['redacted']) - end - - def entry(part_type:, created_at:, author:, body:, attachments:) - writer = author.is_a?(Hash) ? author : {} - - { 'part_type' => part_type, - 'created_at' => stamp(created_at), - 'author_type' => writer['type'], - 'author_name' => writer['name'], - 'author_email' => writer['email'], - 'body' => body, - 'attachment_count' => Array(attachments).size } - end end end end 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 a5abc7587..d37c0b53a 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 @@ -18,6 +18,9 @@ class Ticket < CursorCollection include ContactIdentity include Ticket::Serializer include Ticket::DerivedColumns + # The same thread a conversation publishes, and free here: the parts are + # in the response whether or not anything asks for them. + include Timeline # Intercom accepts 150. This is not that: it is what keeps one page of # tickets, timelines included, a response an agent can hold and an operator @@ -43,6 +46,13 @@ def list_key = 'tickets' def searchable = 'tickets' def max_page_size = MAX_TICKETS_PER_PAGE + # The part bodies are HTML written by end customers, and rendering + # third-party HTML inside Forest is neither safe nor useful (R10). Sent on + # the search, where Intercom does not document it: a parameter it ignores + # costs a query string, while the one it honours saves every row of the + # thread from coming back as markup. + def read_params = { 'display_as' => 'plaintext' } + # 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. @@ -54,6 +64,7 @@ def enrich(records, rows, projection) embed_contact_identity(records, rows, wanted) embed_derived_columns(records, rows, wanted) + embed_timeline(records, rows, wanted) end private @@ -84,6 +95,7 @@ def define_schema define_contact_columns define_derived_columns add_column('part_count', 'Number') + add_column('timeline', 'Json') # Before the attribute columns rather than after: a workspace attribute # whose name lands on a relation is then skipped with a warning, the way # one landing on a column already is. Declared after, it would collide @@ -130,6 +142,20 @@ def define_relations add_many_to_one('contact', foreign_collection: 'IntercomContact', foreign_key: 'contact_id') end + # Costs no request, unlike a conversation's: Intercom returns the parts of + # a ticket in the search response and offers no way to ask it not to, so + # the page pays for them whatever the projection says. Building the thread + # out of them is what is guarded here. + # + # An empty list means an empty thread, and says so -- where a conversation + # read from a listing carries no parts at all and its timeline stays nil, + # which reads as unknown. + def embed_timeline(records, rows, projection) + return unless projection.include?('timeline') + + records.each_with_index { |record, index| rows[index]['timeline'] = build_timeline(record) } + end + # The attribute columns of every ticket type, in union. Read at boot by # `TicketAttributesIntrospector`, which is also where a workspace's own # name is turned into one a Forest query string can carry. An attribute diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/timeline.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/timeline.rb new file mode 100644 index 000000000..8cb3736d2 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/timeline.rb @@ -0,0 +1,53 @@ +module ForestAdminDatasourceIntercom + module Collections + # The thread of a conversation or of a ticket, as a structured list the + # record view can render: who said what, when, and through which kind of + # event. + # + # `part_type` is kept on every entry: an assignment, an internal note and a + # reply are not the same event, and a thread that flattens them reads as an + # exchange that never happened the way it did. Which also means **the + # internal notes of the team are in there**, alongside what the customer + # was told -- that is what a thread is on Intercom, and hiding half of it + # would be the more surprising answer. + # + # Intercom keeps the 500 most recent parts of either resource, so the entry + # count is what is in hand, never necessarily what exists. The two + # collections differ in where they read those parts and in whether anything + # opens the thread before them -- both are hooks. + module Timeline + private + + def build_timeline(entity) + attrs = entity.is_a?(Hash) ? entity : {} + + [opening_entry(attrs)].compact + (parts_of(attrs) || []).map { |part| part_entry(part) } + end + + # What comes before the parts. A conversation opens on its `source`, which + # is not a part at all; a ticket opens on its first part like any other + # event, so there is nothing to prepend. + def opening_entry(_attrs) = nil + + def part_entry(part) + attrs = part.is_a?(Hash) ? part : {} + + entry(part_type: attrs['part_type'], created_at: attrs['created_at'], author: attrs['author'], + body: attrs['body'], attachments: attrs['attachments']) + .merge('id' => stringify_id(attrs['id']), 'redacted' => attrs['redacted']) + end + + def entry(part_type:, created_at:, author:, body:, attachments:) + writer = author.is_a?(Hash) ? author : {} + + { 'part_type' => part_type, + 'created_at' => stamp(created_at), + 'author_type' => writer['type'], + 'author_name' => writer['name'], + 'author_email' => writer['email'], + 'body' => body, + 'attachment_count' => Array(attachments).size } + 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 index 414de6c6a..c5deb04dc 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 @@ -361,6 +361,12 @@ endpoints: part_count: reason: Counted by the agent from the parts the payload carries. source: measured + timeline: + reason: >- + Built by the agent from the parts of the ticket, which the search + endpoint does not read. Filter on `last_reply_at` or on the state + instead. + 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: 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 45c9c52e4..ca03247b5 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 @@ -81,7 +81,7 @@ def stub_search(*records, total: nil, body: nil) answer = { 'type' => 'ticket.list', 'tickets' => records, 'total_count' => total || records.size, 'pages' => { 'type' => 'pages', 'page' => 1 } } - request = stub_request(:post, "#{base}/tickets/search") + request = stub_request(:post, "#{base}/tickets/search").with(query: hash_including({})) request = request.with(body: hash_including(body)) if body request.to_return(json(answer)) end @@ -173,7 +173,7 @@ def columns rows(%w[id]) - expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) .with(body: hash_including('query' => { 'field' => 'created_at', 'operator' => '>', 'value' => '0' })) end @@ -186,7 +186,7 @@ def columns pagination = hash_including('per_page' => described_class::MAX_TICKETS_PER_PAGE) - expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) .with(body: hash_including('pagination' => pagination)) end @@ -251,7 +251,7 @@ def columns # A record detail goes to its own endpoint, which is not the search one. it 'reads one ticket through the record endpoint' do - stub_request(:get, "#{base}/tickets/1").to_return(json(ticket('1'))) + stub_request(:get, "#{base}/tickets/1").with(query: hash_including({})).to_return(json(ticket('1'))) expect(rows(%w[id], condition_tree: leaf('id', operators::EQUAL, '1')).map { |row| row['id'] }).to eq(%w[1]) end @@ -391,12 +391,66 @@ def columns collection.list(nil, filter(condition_tree: leaf('contact_id', operators::EQUAL, 'c1')), %w[id]) - expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) .with(body: hash_including('query' => { 'field' => 'contact_ids', 'operator' => '=', 'value' => 'c1' })) end end + # The exchange itself, which lot 1 paid for and did not publish: the parts + # ride along in every ticket response, so the thread costs no request. + describe 'the thread' do + it 'names who said what, when, and through which kind of event' do + stub_search(ticket('1', parts(comment(at: 1_700_001_000, by: 'Camille', type: 'contact'), + comment(at: 1_700_002_000, by: 'Alice')))) + + expect(rows(%w[id timeline]).first['timeline']) + .to eq([{ 'id' => 'c1700001000', 'part_type' => 'comment', 'created_at' => '2023-11-14T22:30:00Z', + 'author_type' => 'contact', 'author_name' => 'Camille', 'author_email' => nil, + 'body' => 'Je regarde.', 'attachment_count' => 0, 'redacted' => nil }, + { 'id' => 'c1700002000', 'part_type' => 'comment', 'created_at' => '2023-11-14T22:46:40Z', + 'author_type' => 'admin', 'author_name' => 'Alice', 'author_email' => nil, + 'body' => 'Je regarde.', 'attachment_count' => 0, 'redacted' => nil }]) + end + + # An assignment, an internal note and a reply are not the same event, and + # a thread that flattens them reads as an exchange that never happened the + # way it did. Which also means the team's internal notes are in there. + it 'keeps the internal notes and the state changes, each under its own type' do + stub_search(ticket('1', parts(comment(at: 1_700_001_000, part_type: 'note'), + state_change('resolved', at: 1_700_002_000)))) + + expect(rows(%w[id timeline]).first['timeline'].map { |entry| entry['part_type'] }) + .to eq(%w[note ticket_state_updated_by_admin]) + end + + # Empty means empty here, and says so: the parts are in every response, + # unlike a conversation read from a listing, whose timeline stays nil + # because nothing is known. + it 'answers an empty thread on a ticket nothing happened to' do + stub_search(ticket('1')) + + expect(rows(%w[id timeline]).first['timeline']).to eq([]) + end + + it 'builds nothing when the projection does not name it' do + stub_search(ticket('1', parts(comment(at: 1_700_001_000)))) + + expect(rows(%w[id]).first).to eq('id' => '1') + end + + # The bodies are HTML written by end customers, and rendering third-party + # HTML inside Forest is neither safe nor useful (R10). + it 'asks Intercom for plain text rather than markup' do + stub_search(ticket('1')) + + rows(%w[id timeline]) + + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) + .with(query: { 'display_as' => 'plaintext' }) + end + end + describe 'a condition through a relation' do it 'filters on the foreign key the target resolved to' do stub_admins('id' => '493881', 'name' => 'Alice') @@ -404,7 +458,7 @@ def columns rows(%w[id], condition_tree: leaf('admin_assignee:name', operators::EQUAL, 'Alice')) - expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) .with(body: hash_including('query' => { 'field' => 'admin_assignee_id', 'operator' => '=', 'value' => '493881' })) end @@ -417,7 +471,7 @@ def columns rows(%w[id], condition_tree: leaf('admin_assignee:name', operators::EQUAL, 'Alice')) - expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) .with(body: hash_including('query' => { 'operator' => 'OR', 'value' => [{ 'field' => 'admin_assignee_id', 'operator' => '=', @@ -434,7 +488,7 @@ def columns stub_admins('id' => '493881', 'name' => 'Alice') expect(rows(%w[id], condition_tree: leaf('admin_assignee:name', operators::EQUAL, 'Zoe'))).to be_empty - expect(WebMock).not_to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).not_to have_requested(:post, %r{#{base}/tickets/search}) end it 'counts none of them either, without a request' do @@ -445,7 +499,7 @@ def columns operators::EQUAL, 'Zoe')), aggregation) expect(counted).to eq([{ 'group' => {}, 'value' => 0 }]) - expect(WebMock).not_to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).not_to have_requested(:post, %r{#{base}/tickets/search}) end # A relation group nested inside the tree the agent assembled: a scope, a @@ -459,7 +513,7 @@ def columns collection.list(nil, filter(condition_tree: tree), %w[id]) - expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) .with(body: hash_including('query' => { 'operator' => 'AND', 'value' => [{ 'field' => 'category', 'operator' => '=', @@ -486,7 +540,7 @@ def columns collection.list(nil, filter(condition_tree: tree), %w[id]) - expect(WebMock).to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).to have_requested(:post, %r{#{base}/tickets/search}) .with(body: hash_including('query' => { 'operator' => 'AND', 'value' => [{ 'field' => 'category', 'operator' => '=', @@ -512,7 +566,7 @@ def columns collection.list(nil, filter(condition_tree: tree), %w[id]) - expect(WebMock).to(have_requested(:post, "#{base}/tickets/search").with do |request| + expect(WebMock).to(have_requested(:post, %r{#{base}/tickets/search}).with do |request| query = JSON.parse(request.body)['query'] query['value'].size == 15 && query['value'].last['operator'] == 'OR' && query['value'].last['value'].size == 3 @@ -525,7 +579,7 @@ def columns leaf('admin_assignee:name', operators::EQUAL, 'Zoe')) expect(collection.list(nil, filter(condition_tree: tree), %w[id])).to be_empty - expect(WebMock).not_to have_requested(:post, "#{base}/tickets/search") + expect(WebMock).not_to have_requested(:post, %r{#{base}/tickets/search}) end # Fifteen conditions per group is Intercom's limit, and a relation reaches From 6880e2c05ae5ec3afc9c9004c4ac37e7bc95c39a Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Mon, 7 Sep 2026 17:21:53 +0200 Subject: [PATCH 5/5] fix(intercom): skip a custom attribute a column already carries Measured on a real workspace: a custom contact attribute named `id`. The toolkit refuses a field declared twice, so the agent did not boot -- and had it accepted the column, the serializer would have written the attribute over the record's own key. Tickets already guarded against this since lot 1. The guard moves to a shared module the three collections use, and Contacts and Companies declare their relations before registering these columns, so a name landing on a relation is skipped the same way. Same module fixes a second thing on the two new collections: a custom date arrives as epoch seconds like every other Intercom date, and a Date column handed an integer renders as one. Co-Authored-By: Claude Opus 5 (1M context) --- .../collections/company.rb | 10 +-- .../collections/company/serializer.rb | 10 +-- .../collections/contact.rb | 11 +-- .../collections/contact/serializer.rb | 11 +-- .../collections/custom_attributes.rb | 69 +++++++++++++++++++ .../collections/ticket.rb | 24 +------ .../collections/ticket/serializer.rb | 33 ++------- .../collections/company_spec.rb | 20 ++++++ .../collections/contact_spec.rb | 36 ++++++++++ 9 files changed, 149 insertions(+), 75 deletions(-) create mode 100644 packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/custom_attributes.rb diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb index cb8563ef5..5377cd4dc 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company.rb @@ -20,6 +20,7 @@ module Collections # operators looking at a list at the same time. class Company < OffsetCollection include Company::Serializer + include CustomAttributes # The column each lookup is written on, and the query parameter Intercom # answers it under. They happen to share a name; keeping the mapping @@ -49,8 +50,11 @@ def define_schema add_column('name', 'String') define_profile_columns define_activity_columns - define_attribute_columns + # Before the attribute columns, so an attribute whose name lands on the + # relation is skipped with a warning rather than taking the boot with + # it. add_one_to_many('contacts', foreign_collection: 'IntercomContact', origin_key: 'company_id') + register_attribute_columns end def define_profile_columns @@ -75,9 +79,7 @@ def define_activity_columns # Typed from `GET /data_attributes?model=company`, and unfilterable for # the same reason as everything else here: this collection is looked up, # not searched. - def define_attribute_columns - @attributes.each { |attribute| add_column(attribute.column_name, attribute.column_type) } - end + def attribute_kind = 'company' end end end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb index 4b9eb7fc3..c449d1070 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/company/serializer.rb @@ -13,7 +13,7 @@ def serialize(company) 'plan_name' => plan['name'], 'user_count' => attrs['user_count'], 'session_count' => attrs['session_count'] - ).merge(dates_of(attrs)).merge(attribute_columns_for(attrs)) + ).merge(dates_of(attrs)).merge(attribute_values(attrs['custom_attributes'])) end private @@ -38,14 +38,6 @@ def dates_of(attrs) 'remote_created_at' => stamp(attrs['remote_created_at']) } end - - # Nil rather than absent for an attribute the company does not carry: - # the column exists on every row. - def attribute_columns_for(attrs) - values = attrs['custom_attributes'].is_a?(Hash) ? attrs['custom_attributes'] : {} - - @attributes.to_h { |attribute| [attribute.column_name, values[attribute.name]] } - end end end end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb index 8db270d4b..871591733 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact.rb @@ -24,6 +24,7 @@ module Collections # Long by line count only: most of it declares the columns, one call each. class Contact < CursorCollection # rubocop:disable Metrics/ClassLength include Contact::Serializer + include CustomAttributes # `/contacts/search` demands a query, so a read with no condition of its # own -- a list view asking for an order -- sends the least noisy @@ -63,8 +64,12 @@ def define_schema define_date_columns define_reachability_columns define_device_columns - define_attribute_columns + # Before the attribute columns rather than after: a workspace attribute + # whose name lands on a relation is then skipped with a warning, the way + # one landing on a column already is. Declared after, it would collide + # and take the boot with it. define_relations + register_attribute_columns end def define_identity_columns @@ -117,9 +122,7 @@ def define_device_columns # offers no filter it has not seen work. `api_writable` travels on the # introspected attribute for lot 4b, not on the column -- everything here # is read-only. - def define_attribute_columns - @attributes.each { |attribute| add_column(attribute.column_name, attribute.column_type) } - end + def attribute_kind = 'contact' # The owner is a teammate, read whole in one request, and # `/contacts/search` filters on the key -- so that relation is readable, diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb index 49283736b..509351034 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/contact/serializer.rb @@ -13,7 +13,7 @@ def serialize(contact) native(attrs) .merge(account_of(attrs['companies'])) .merge(location_of(attrs['location'])) - .merge(attribute_columns_for(attrs)) + .merge(attribute_values(attrs['custom_attributes'])) end private @@ -97,15 +97,6 @@ def location_of(location) 'location_city' => attrs['city'] } end - # Nil rather than absent for an attribute the contact does not carry: - # the column exists on every row, and an absent key would read as a - # record missing it. - def attribute_columns_for(attrs) - values = attrs['custom_attributes'].is_a?(Hash) ? attrs['custom_attributes'] : {} - - @attributes.to_h { |attribute| [attribute.column_name, values[attribute.name]] } - end - def domain_of(email) email.to_s[/@(.+)\z/, 1] end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/custom_attributes.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/custom_attributes.rb new file mode 100644 index 000000000..8437079d6 --- /dev/null +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/custom_attributes.rb @@ -0,0 +1,69 @@ +module ForestAdminDatasourceIntercom + module Collections + # The columns a workspace's own attributes become, and the two things that + # go wrong if they are published as they come. + # + # **A name can land on a column the collection already carries.** Measured + # on a real workspace: a custom contact attribute named `id`. Adding it + # would raise on the second declaration -- the toolkit refuses a field twice + # -- and, if it did not, the serializer would write the attribute where the + # operator expects the record's key. It is skipped, and the log names which + # one, since the fix is on Intercom's side. + # + # Relations count as taken names, which is why every collection here + # declares them **before** registering these columns. + # + # **A date arrives as epoch seconds**, like every other Intercom date, and a + # Date column that receives an integer renders as one. + module CustomAttributes + private + + def register_attribute_columns + @attribute_columns = @attributes.reject { |attribute| collides?(attribute) } + @attribute_columns.each { |attribute| add_column(attribute.column_name, attribute.column_type) } + end + + def attribute_columns = @attribute_columns || [] + + # What the log calls these, which is the workspace's own vocabulary: a + # ticket attribute is declared per ticket type, a contact attribute per + # model. + def attribute_kind = raise(NotImplementedError, "#{self.class} did not implement attribute_kind") + + def collides?(attribute) + return false unless fields.key?(attribute.column_name) + + ForestAdminDatasourceIntercom.logger.warn( + "[forest_admin_datasource_intercom] #{name} skips the #{attribute_kind} attribute " \ + "#{attribute.name.inspect}: a native column or relation already carries the name " \ + "#{attribute.column_name.inspect}, and overwriting it would show the attribute where the operator " \ + 'expects the record field. Rename it in Intercom to publish it.' + ) + true + end + + # The value of each published attribute, read under the name the workspace + # gave it and written under the column name the schema publishes -- the + # two differ whenever the first could not travel through a Forest query + # string. + # + # Nil rather than absent for an attribute the record does not carry: the + # column exists on every row, and an absent key would read as a record + # missing it. + def attribute_values(values) + held = values.is_a?(Hash) ? values : {} + + attribute_columns.to_h do |attribute| + [attribute.column_name, coerce_attribute(held[attribute.name], attribute)] + end + end + + def coerce_attribute(value, attribute) + return nil if value.nil? + return stamp(value) if attribute.column_type == 'Date' && value.is_a?(Numeric) + + value + end + end + end +end 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 d37c0b53a..533d5f3ed 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 @@ -18,6 +18,7 @@ class Ticket < CursorCollection include ContactIdentity include Ticket::Serializer include Ticket::DerivedColumns + include CustomAttributes # The same thread a conversation publishes, and free here: the parts are # in the response whether or not anything asks for them. include Timeline @@ -158,27 +159,8 @@ def embed_timeline(records, rows, projection) # The attribute columns of every ticket type, in union. Read at boot by # `TicketAttributesIntrospector`, which is also where a workspace's own - # name is turned into one a Forest query string can carry. An attribute - # landing on a native column is skipped rather than overwriting it. - def register_attribute_columns - @attribute_columns = @attributes.reject { |attribute| collides?(attribute) } - @attribute_columns.each { |attribute| add_column(attribute.column_name, attribute.column_type) } - end - - def collides?(attribute) - return false unless fields.key?(attribute.column_name) - - ForestAdminDatasourceIntercom.logger.warn( - "[forest_admin_datasource_intercom] #{name} skips the ticket attribute #{attribute.name.inspect}: a " \ - "native column or relation already carries the name #{attribute.column_name.inspect}, and overwriting " \ - 'it would show the attribute where the operator expects the ticket field.' - ) - true - end - - def attribute_columns - @attribute_columns || [] - end + # name is turned into one a Forest query string can carry. + def attribute_kind = 'ticket' end end end diff --git a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket/serializer.rb b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket/serializer.rb index d547fefb0..a094bbf20 100644 --- a/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket/serializer.rb +++ b/packages/forest_admin_datasource_intercom/lib/forest_admin_datasource_intercom/collections/ticket/serializer.rb @@ -7,6 +7,11 @@ class Ticket < CursorCollection module Serializer protected + # Intercom keys the attribute values by **name**, which is what lets a + # single collection display the union of every ticket type's -- and what + # stops it from filtering on them, the filter being written by an id + # that differs from one type to the next. A ticket of another type + # simply does not carry the key, and the column reads as empty. def serialize(ticket) attrs = ticket.is_a?(Hash) ? ticket : {} @@ -14,7 +19,7 @@ def serialize(ticket) .merge(state_of(attrs)) .merge(type_of(attrs['ticket_type'])) .merge(contact_columns_for(attrs)) - .merge(attribute_values_of(attrs['ticket_attributes'])) + .merge(attribute_values(attrs['ticket_attributes'])) .merge(derived_columns_for(attrs)) end @@ -49,32 +54,6 @@ def type_of(ticket_type) { 'ticket_type_id' => stringify_id(attrs['id']), 'ticket_type_name' => attrs['name'] } end - - # Intercom keys the values by attribute **name**, which is what lets a - # single collection display the union of every type's attributes -- and - # what stops it from filtering on them, since the filter is written by id - # and the id differs from one type to the next. - # - # The value is read under the name the workspace gave it and written - # under the column name the schema publishes; the two differ whenever the - # first could not travel through a Forest query string. - # - # A ticket of another type simply does not carry the key: the column - # reads as absent rather than as empty. - def attribute_values_of(values) - held = values.is_a?(Hash) ? values : {} - - attribute_columns.to_h { |attribute| [attribute.column_name, coerce(held[attribute.name], attribute)] } - end - - # A date attribute comes back as epoch seconds like every other Intercom - # date; the rest is handed over as it came. - def coerce(value, attribute) - return nil if value.nil? - return stamp(value) if attribute.column_type == 'Date' && value.is_a?(Numeric) - - value - end end end end diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb index 6151d1298..381399d05 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/company_spec.rb @@ -115,6 +115,26 @@ def rows(projection = %w[id], **options) expect(collection.fields['arr'].filter_operators).to be_empty end + it 'skips an attribute whose name a native column already carries, and says which' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_data_attributes('company', { 'name' => 'name', 'data_type' => 'string', 'custom' => true, + 'api_writable' => true, 'archived' => false }) + stub_page(company('co1', 'custom_attributes' => { 'name' => 'Not the account name' }), + page: 1, per_page: 15) + + expect(rows(nil, page: page(0, 15)).first['name']).to eq('Company co1') + expect(ForestAdminDatasourceIntercom.logger) + .to have_received(:warn).with(/skips the company attribute "name"/) + end + + it 'reads a date attribute as a date' do + stub_data_attributes('company', { 'name' => 'renewal', 'data_type' => 'date', 'custom' => true, + 'api_writable' => true, 'archived' => false }) + stub_page(company('co1', 'custom_attributes' => { 'renewal' => 1_700_000_000 }), page: 1, per_page: 15) + + expect(rows(%w[id renewal], page: page(0, 15)).first['renewal']).to eq('2023-11-14T22:13:20Z') + end + it 'reads its value off the payload' do stub_page(company('co1', 'custom_attributes' => { 'arr' => 12_000 })) diff --git a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb index 7111bb263..d461612fb 100644 --- a/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb +++ b/packages/forest_admin_datasource_intercom/spec/forest_admin_datasource_intercom/collections/contact_spec.rb @@ -154,6 +154,42 @@ def rows(projection = %w[id], **options) expect(collection.fields['paid_subscriber'].is_sortable).to be(false) end + # Measured on a real workspace, and it took the agent's boot with it: the + # toolkit refuses a field declared twice, so an attribute landing on a + # native column has to be skipped -- and skipped rather than renamed, + # since the operator expects the record's own key under `id`. + it 'skips an attribute whose name a native column already carries, and says which' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_data_attributes('contact', { 'name' => 'id', 'data_type' => 'string', 'custom' => true, + 'api_writable' => true, 'archived' => false }) + stub_list(contact('1', 'custom_attributes' => { 'id' => 'erp-42' })) + + expect { collection }.not_to raise_error + expect(rows(nil).first['id']).to eq('1') + expect(ForestAdminDatasourceIntercom.logger) + .to have_received(:warn).with(/skips the contact attribute "id"/) + end + + # Relations are declared before these columns for exactly this reason. + it 'skips one landing on a relation name too' do + allow(ForestAdminDatasourceIntercom.logger).to receive(:warn) + stub_data_attributes('contact', { 'name' => 'company', 'data_type' => 'string', 'custom' => true, + 'api_writable' => true, 'archived' => false }) + + expect(collection.fields['company']) + .to be_a(ForestAdminDatasourceToolkit::Schema::Relations::ManyToOneSchema) + end + + # A custom date arrives as epoch seconds like every other Intercom date, + # and a Date column handed an integer renders as one. + it 'reads a date attribute as a date' do + stub_data_attributes('contact', { 'name' => 'renewal', 'data_type' => 'date', 'custom' => true, + 'api_writable' => true, 'archived' => false }) + stub_list(contact('1', 'custom_attributes' => { 'renewal' => 1_700_000_000 })) + + expect(rows(%w[id renewal]).first['renewal']).to eq('2023-11-14T22:13:20Z') + end + it 'reads its value off the payload, nil where the contact carries none' do stub_list(contact('1', 'custom_attributes' => { 'paid_subscriber' => true }), contact('2'))