Skip to content

feat(datasource intercom): relations of the reference tier (lot 2.5) - #385

Merged
christophebrun-forest merged 3 commits into
feat/datasource-intercomfrom
feat/intercom-tier-c-relations
Sep 7, 2026
Merged

feat(datasource intercom): relations of the reference tier (lot 2.5)#385
christophebrun-forest merged 3 commits into
feat/datasource-intercomfrom
feat/intercom-tier-c-relations

Conversation

@christophebrun-forest

@christophebrun-forest christophebrun-forest commented Sep 4, 2026

Copy link
Copy Markdown
Member

Lot 2.5 of PRD-1111PRD-1145. Branches off feat/datasource-intercom and merges back into it.

Declares the relations between the collections this datasource already serves, and answers them. Lot 1 shipped none, on the ground that a relation whose target collection is missing makes the schema refuse to boot — true of Contacts and Companies only, while IntercomAdmin, IntercomTeam, IntercomTicketState and IntercomTicketType all shipped in that same lot. The visible consequence: an assignee, a state and a type were raw ids, and a team was an array of ids with no link to its teammates.

What sizes this change

Publishing a many-to-one is not free. The agent marks it filterable as soon as any column of its target is (GeneratorField#build_many_to_one_schema), so the interface offers assignee:name the moment the relation exists — and the datasource cannot opt out: making the relation unfilterable would mean stripping the filters off Admin and Team themselves. Left unanswered, that filter reaches a translator refusing every traversing field, which is the interface offering a filter this package then refuses.

So the traversal is the bulk of the diff, and the declarations are a handful of lines.

What it does

  • Eight many-to-one relations towards Admin, Team, TicketState and TicketType, declared before the ticket-attribute columns so a workspace attribute whose name lands on a relation is skipped with a warning rather than colliding and taking the boot with it.
  • IntercomTeamMembership, synthesized from GET /teams: Intercom carries the membership on both sides (teams[].admin_ids, admins[].team_ids) and exposes no resource for the pair, while a many-to-many needs a collection to travel through. One record per pair, keyed teamId:adminId, read-only — Intercom writes none. Teams and Admins now navigate to each other both ways.
  • admin_names / team_names replace the arrays of ids on Team and Admin: one readable form plus a relation to navigate, rather than two ways to read one fact. Read only when a projection asks for them, and a token that cannot read the other side costs the column, never the page or the relation.
  • One label per object on a ticket: state_category and state_external_label are dropped — a hop away on the state relation, and neither was ever filterable, so no segment, scope or saved filter could rest on them. state_label and ticket_type_name stay, a list view having to read without a join.
  • A projection through a relation reads its target once per page, never once per row, and nests the row under the relation name. An id naming no record reads as no record, not as a broken row.
  • A condition through a relation is resolved by the target itself — over every record it holds, not over a page — and becomes a condition on the foreign key. Exact, with three edges, all refused by name rather than approximated:
    • Intercom takes no membership operator on these fields, so N matches become an OR of N equalities, which counts against the 15 conditions a group allows. Past that: refused, naming the relation the operator filtered on rather than the key it resolved to.
    • A condition the target matched nothing with names no row, and the DSL cannot say so: the search is skipped entirely instead of being 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 before the target is read, saying which of the two it is: read and navigate, not filter.

Decisions taken while implementing

  • A nil projection now returns only the declared columns. The raw membership ids stay on the serialized row — they are what the names are read from — but are no longer published: project slices on the columns.
  • A relation's filterability is checked before the target is read. A filter on state:category used to read /ticket_states for nothing before refusing.
  • The unknown-relation refusal covers the many-to-many too. A scope written on admins:name is refused saying which relations are filtered through, rather than claiming the relation does not exist.

Known, and documented rather than discovered

  • A relation reads its target undecorated, so a permission scope or segment on the target does not narrow what a relation resolves. That is how a native datasource behaves — it joins the table without applying the scopes of the collection mapped to it — but it belongs in the README.
  • A row whose foreign key is null matches no relation filter, the way a join drops it, negated filters included. A ticket with no assignee is not "assigned to someone other than Marie".
  • state_id is still to probe on /tickets/search: the measured table carries no filter on a state id, so the state relation navigates without translating into a filter. In the refusal table and in the README, not in an assumption.

Verification

  • 418 examples, 0 failures, 100% line coverage on the package.
  • RuboCop clean across the 899 files of the repo.
  • Covered: the refusals, the id-type match on both ends of every relation, the 15-leaf ceiling, matches-nothing without a request, relation conditions nested inside and / or next to native ones, and the real related-list path through Utils::Collection.list_relation.
  • Every payload is hand-written from the OpenAPI 2.16 spec, never captured from a workspace.

README: a ## Relations section (the table, the synthesized membership, the read cost, the null semantics) and a ### Through a relation subsection under Filtering.

🤖 Generated with Claude Code

Note

Add reference-tier relations and IntercomTeamMembership to Intercom datasource

  • Adds a shared relations.rb module that lets collections declare read-only many-to-one and many-to-many relations, project nested records, and rewrite relation filters into foreign-key conditions.
  • Introduces team_membership.rb as a synthesized collection to support the many-to-many relation between IntercomTeam and IntercomAdmin.
  • Updates Ticket, Admin, Team, and Conversation collections to declare relations such as admin_assignee, teams, and closed_by, replacing raw ID lists with derived admin_names and team_names columns.
  • Risk: Removes the team_ids, admin_ids, state_category, and state_external_label columns from the Intercom schemas; any existing consumers relying on these fields will break.

Macroscope summarized e1ae9b4.

Declares the relations between the collections this datasource already
serves, and answers them (lot 2.5, PRD-1145).

Publishing a many-to-one is not free: the agent marks it filterable as
soon as any column of its target is, so the interface offers
`assignee:name` the moment the relation exists. The traversal is
therefore the bulk of this change, not the declarations.

* eight many-to-one relations towards Admin, Team, TicketState and
  TicketType, whose targets all shipped with lot 1;
* IntercomTeamMembership, synthesized from GET /teams: Intercom carries
  the membership on both sides and exposes no resource for the pair,
  while a many-to-many needs a collection to travel through. Read-only,
  Intercom writing none;
* admin_names and team_names replace the arrays of ids on Team and
  Admin, read only when a projection asks for them;
* one label per object on a ticket: state_category and
  state_external_label are dropped, both being a hop away on the state
  relation and neither having ever been filterable;
* a projection through a relation reads its target once per page; a
  condition through one is resolved by the target, exactly, and becomes
  a condition on the foreign key. Over fifteen matching ids, or on a key
  the endpoint does not filter, it is refused by name rather than
  approximated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@qltysh

qltysh Bot commented Sep 4, 2026

Copy link
Copy Markdown

All good ✅

@qltysh

qltysh Bot commented Sep 4, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

Modified Files with Diff Coverage (12)

RatingFile% DiffUncovered Line #s
New file Coverage rating: A
...rcom/lib/forest_admin_datasource_intercom/collections/admin.rb100.0%
New file Coverage rating: A
...t_admin_datasource_intercom/query/condition_tree_translator.rb100.0%
New file Coverage rating: A
...orest_admin_datasource_intercom/collections/team_membership.rb100.0%
New file Coverage rating: A
...ercom/lib/forest_admin_datasource_intercom/collections/team.rb100.0%
New file Coverage rating: A
...b/forest_admin_datasource_intercom/collections/conversation.rb100.0%
New file Coverage rating: A
.../lib/forest_admin_datasource_intercom/collections/relations.rb100.0%
New file Coverage rating: A
..._admin_datasource_intercom/collections/fetch_all_collection.rb100.0%
New file Coverage rating: A
...ce_intercom/lib/forest_admin_datasource_intercom/datasource.rb100.0%
New file Coverage rating: A
...orest_admin_datasource_intercom/collections/base_collection.rb100.0%
New file Coverage rating: A
...est_admin_datasource_intercom/collections/cursor_collection.rb100.0%
New file Coverage rating: A
...oolkit/lib/forest_admin_datasource_toolkit/utils/collection.rb100.0%
New file Coverage rating: A
...com/lib/forest_admin_datasource_intercom/collections/ticket.rb100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Comment thread packages/forest_admin_datasource_intercom/README.md Outdated
Review of the relations lot, six items.

A relation group is now told apart from a group the operator wrote, and
inlined into a parent that aggregates the same way: the level it added
was one nothing budgeted against Intercom's two, so a scope plus a
"match any" filter carrying one relation condition was refused for a
nesting the operator could not find in their own filter. Inlining trades
depth for width and Intercom bounds both, so past fifteen conditions the
nested form is kept. Where a group is still refused for depth, the
message names the relation that expanded into it rather than asking to
flatten a nesting nobody wrote.

Relations are read one request per target collection instead of one per
relation: a ticket's state and previous_state are one read of
/ticket_states, over the ids both of them name, and each relation is
still nested with the columns it asked for rather than the union.

An enrichment guarded on a column being named left that column nil under
a projection naming nothing, which project reads as every declared
column: admin_names and team_names came back nil on the one listing that
publishes them.

get_field_schema raised a bare ForestException on the three field names
a request can get wrong, which the error translator maps to a 500. They
are ValidationErrors now, so a condition reaching through a relation the
toolkit cannot traverse -- a scope written on a many-to-many, say -- is
the 400 it always was in intent.

README: the permission note separated the enrichment, which is guarded,
from the relation read, which is not; the cost of a relation stated per
target collection; the page cap that bounds "read whole"; and the
inlining above.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two failures a many-to-many related list could reach, neither of
which any tier reported.

`Utils::Collection.list_relation` unwraps a through row by the name
of the relation it travels through, and a target the foreign
collection no longer answers leaves a nil in its place. Deleted,
outside the caller's reach, or past the page cap of a datasource
reading its targets whole: the row reached the JSON:API serializer,
which reads a record by key, as a 500. A join whose other side is
gone yields no row, not an empty one, so the mapping compacts.

The other is an order asked for and not honoured. The agent hands
the through collection the sort written against the collection the
relation reaches -- `Filter#nest` prefixes the condition tree and
not the sort -- so a related list of teammates ordered by name
reached `FetchAllCollection` with a clause it carries no column
for, and dropped it without a word. Every other order this
datasource cannot honour is reported; this one now is too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

aggregate_relation counts through rows even when their nested target resolves to nil, so /count-related reports memberships that list_relation has already removed. This makes relationship counts and pagination incorrect for deleted or inaccessible targets; apply the same target-existence filtering before aggregating.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_datasource_toolkit/lib/forest_admin_datasource_toolkit/utils/collection.rb around line 197:

`aggregate_relation` counts through rows even when their nested target resolves to `nil`, so `/count-related` reports memberships that `list_relation` has already removed. This makes relationship counts and pagination incorrect for deleted or inaccessible targets; apply the same target-existence filtering before aggregating.

@christophebrun-forest
christophebrun-forest merged commit 3398782 into feat/datasource-intercom Sep 7, 2026
33 checks passed
@christophebrun-forest
christophebrun-forest deleted the feat/intercom-tier-c-relations branch September 7, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant