feat(zendesk): add Zendesk datasource package

[PMerlet]

Summary

New forest_admin_datasource_zendesk package that surfaces Zendesk Tickets, Users, Organizations, and ticket Comments as Forest Admin collections — same architectural pattern as the existing forest_admin_datasource_active_record / _mongoid / _rpc siblings, mirroring the Node @forestadmin/datasource-* ecosystem.

• 4 collections with schema-level intra-datasource relations: Ticket → requester/assignee/organization/comments, Comment → author/ticket, User → organization/requested_tickets, Organization → users/tickets.
• Custom fields: discovers admin-defined ticket_fields, user_fields, organization_fields at boot and exposes them as real Forest columns. Translator rewrites custom-column names to Zendesk Search syntax.
• Timezone-aware Date filters: Forest's caller timezone is threaded through the translator; bare Date values are interpreted as start-of-day in the caller's TZ then converted to UTC (DST-aware).
• Loud-vs-graceful error handling: critical paths (search, count, fetch_ticket_comments) raise APIError; enrichment paths (bulk user/org lookups, schema introspection) log a warning and degrade safely.
• Synthetic single-PK on Comments (<comment_id>-<ticket_id> String) to side-step forest_admin_rails 1.26.2's URL constraint, which rejects the | character used by the toolkit's native composite-key pack_id. Filed-this-as-a-bug-worth-fixing-upstream note in the relevant code comment.

What's NOT in this PR (deliberate v1 scope)

• Read-only — no create/update/delete on any collection. The Compliance handoff workflow (update ticket status / reassign group) is the obvious next add.
• No caching — every list page hits Zendesk's Search API + bulk user/org lookups. Add Rails.cache wrapping in a follow-up if rate limits become a concern.
• Zendesk Search's 1000-result hard cap is not surfaced in pagination yet (cursor-based Incremental Export API would be the correct path for large datasets).
• No native OR-aggregator translation (raises loudly on unsupported operators).

Test plan

• cd packages/forest_admin_datasource_zendesk && bundle install && bundle exec rspec — 131 examples, 0 failures
• Coverage ≥ 90% line and branch (98.5% line / 90.7% branch with WebMock blocking real net)
• Manually verified end-to-end against a live Zendesk dev instance (browse tickets, requester join, conversation thread via Ticket → comments related list, comment detail page via synthetic PK)
• Reviewer to confirm: gemspec metadata, dependency pins, SchemaEmitter compatibility with toolkit ≥ 1.26
• Reviewer to confirm: package layout matches conventions (Zeitwerk, Rakefile, .rspec)

🤖 Generated with Claude Code

Note

Add Zendesk datasource package with collections for tickets, users, organizations, and comments

• Introduces a new forest_admin_datasource_zendesk gem that wraps the Zendesk API as a Forest Admin datasource, supporting list, count, create, update, and delete for tickets, users, organizations, and comments.
• Client wraps zendesk_api with consistent error handling: critical paths raise APIError, non-critical paths log and return safe defaults.
• ConditionTreeTranslator converts Forest condition trees to Zendesk Search query strings, with timezone-aware date handling and custom field mapping.
• CustomFieldsIntrospector fetches Zendesk custom field definitions at boot and produces typed ColumnSchema entries for tickets, users, and organizations.
• The Comment collection is read-only and only returns results when a ticket_id scope is present in the filter; it uses a synthetic composite <comment_id>-<ticket_id> primary key.
• Release pipeline in .releaserc.js and CI in .github/workflows/build.yml are extended to build, publish, and report coverage for the new gem.

Changes since #288 opened

• Refactored collections.Ticket to use four new mixin modules and changed comments from a OneToMany relation to a read-only structured array column [22d782e]
• Modified ForestAdminDatasourceZendesk::Query::ConditionTreeTranslator.format_string to always quote and escape string filter values [22d782e]
• Refactored ForestAdminDatasourceZendesk::Client to extract write and introspection operations into separate modules and added bulk ticket fetching [22d782e]
• Centralized Zendesk query string building into collections.BaseCollection.build_zendesk_query and refactored dependent methods to use it [22d782e]
• Removed Comment collection registration from ForestAdminDatasourceZendesk::Datasource [22d782e]
• Refactored collections.BaseCollection.project to use to_h for projection filtering [22d782e]
• Updated test coverage for ForestAdminDatasourceZendesk::Query::ConditionTreeTranslator value quoting [22d782e]
• Updated packages/forest_admin_datasource_zendesk/collections/ticket_spec.rb to reflect schema and behavior changes [22d782e]
• Added packages/forest_admin_datasource_zendesk/forest_admin_datasource_zendesk.gemspec to Gemspec/RequireMFA exclusions and set metadata rubygems_mfa_required to false [22d782e]
• Removed per-package RuboCop configuration and cleaned up documentation comments [22d782e]
• Wrapped RSpec specs in ForestAdminDatasourceZendesk module with included ForestAdminDatasourceToolkit::Components::Query and locally-rebound constants [ed28d88]
• Refactored RSpec expectations from pre-invocation expect(...).to receive to post-invocation allow(...).to receive with expect(...).to have_received assertions [ed28d88]
• Added enhanced assertions in Collections::Ticket spec for client interaction verification [ed28d88]
• Introduced leaf_class and branch_class helpers in Query::ConditionTreeTranslator spec [ed28d88]
• Added packages/forest_admin_datasource_zendesk/spec/**/* to Metrics/ModuleLength exclusion list in .rubocop.yml [ed28d88]
• Reformatted test setup code for readability in Client and Schema::CustomFieldsIntrospector specs [ed28d88]

^{Macroscope summarized 4ac39d5.}

[qltysh]

4 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many parameters (count = 4): put_resource	2
qlty	Duplication	Found 15 lines of similar code in 2 locations (mass = 94)	2

[qltysh]

Function with many parameters (count = 4): aggregate [qlty:function-parameters]

[macroscopeapp]

🟢 Low query/condition_tree_translator.rb:59

The special requester: mapping on line 63 only applies to the EQUAL operator. For NOT_EQUAL, IN, or NOT_IN on requester_email, the code falls through to the generic path and emits the unmappable field name requester_email instead of requester, producing malformed queries like -requester_email:value or requester_email:val1 requester_email:val2.

       def translate_leaf(leaf)
         field = mapped_field(leaf.field)
         value = leaf.value
 
-        return "requester:#{format_value(value)}" if leaf.field == 'requester_email' && leaf.operator == Operators::EQUAL
+        field = 'requester' if leaf.field == 'requester_email'
 
         case leaf.operator

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/query/condition_tree_translator.rb around lines 59-64:

The special `requester:` mapping on line 63 only applies to the `EQUAL` operator. For `NOT_EQUAL`, `IN`, or `NOT_IN` on `requester_email`, the code falls through to the generic path and emits the unmappable field name `requester_email` instead of `requester`, producing malformed queries like `-requester_email:value` or `requester_email:val1 requester_email:val2`.

Evidence trail:
packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/query/condition_tree_translator.rb at REVIEWED_COMMIT:
- Line 58: `field = mapped_field(leaf.field)`
- Line 63: `return "requester:#{format_value(value)}" if leaf.field == 'requester_email' && leaf.operator == Operators::EQUAL` - special case only for EQUAL
- Lines 65-68: generic case statement uses `field` variable for NOT_EQUAL, IN, NOT_IN
- Lines 87-89: `mapped_field` returns `@custom_fields[field] || field` - returns field name unchanged if not in custom_fields

[qltysh]

Found 15 lines of similar code in 2 locations (mass = 94) [qlty:similar-code]

[qltysh]

Found 15 lines of similar code in 2 locations (mass = 94) [qlty:similar-code]

[matthv]

The comment density across this package feels much higher than the rest of the monorepo (compare with active_record DS which is nearly comment-free).
A lot just restate the code. The # Error policy: block at the top doesn't add anything must_succeed / best_effort don't already say.

[matthv]

Looks like the Rakefile here isn't actually used (CI and release both bypass rake). Worth dropping?

[matthv]

Since this is a new datasource, the "previous version" referenced here only ever existed during this PR's development. Once merged, the comment loses its anchor

[matthv]

The invariant is already covered by ticket_spec.rb:130 (preserves ascending=false even when both symbol and string keys are present). The test name says enough. This comment, and the duplicated # Regression: block in the test itself, can both be dropped.

[matthv]

This line triggers one GET /tickets/:id call per id. With id IN [1..50], that's 50 sequential HTTP round-trips (multiple seconds of latency).

Zendesk has a /tickets/show_many?ids=… endpoint that returns up to 100 tickets in a single call. The bulk_show_many helper in client.rb already implements that pattern for users and organizations (see fetch_users_by_ids / fetch_organizations_by_ids). Worth applying the same approach to tickets to stay consistent and avoid the latency cliff.

[matthv]

Reconsidering the Comment collection design

The log + return [] when list is called without a ticket scope (lines 19-27) is the visible symptom of a deeper layering issue. Walking through the current state:

1. Comment is registered as a top-level collection, so it appears in the Forest sidebar alongside Ticket / User / Organization.
2. When a user clicks it, Forest calls list(caller, filter, projection) with no ticket_id in the filter.
3. The collection logs an info line nobody will ever read, returns [], and the user sees an empty grid with no explanation.
4. To get a stable URL for an individual comment via Ticket > comments, the schema invents a synthetic <comment_id>-<ticket_id> String PK to dodge forest_admin_rails 1.26.2's URL constraint.

That's a lot of complexity (sidebar entry, scope resolution, synthetic PK, custom extract_field_lookup, Branch-walking logic) to support a use case Zendesk's API doesn't actually expose: standalone comment browsing. There's no /comments/{id} endpoint, comments are always fetched as /tickets/{id}/comments.

Why "just hide the sidebar entry" doesn't work
Checked the publication decorator: remove_collection cascades. published? in PublicationCollectionDecorator:79-85 will unpublish Ticket > comments as soon as Comment is removed. The toolkit doesn't currently expose a "hide from sidebar but keep accessible via relation" flag at the collection level.

Proposed simpler design: drop the Comment collection

Comments are read-only here (no create/update/delete/aggregate). The only valid access pattern is "give me the comments of this ticket." An embedded structured field on Ticket models that cleanly.

add_external_relation in the customizer layer does this, but it's the wrong layer for a datasource gem. We can do the same thing natively at the datasource layer because ColumnSchema#column_type accepts a structured [hash] shape and Ticket#list already controls projection-aware fetching.

# In Ticket#initialize:
COMMENT_THREAD_SCHEMA = {
 'id'           => 'Number',
'body'         => 'String',
'html_body'    => 'String',
'public'       => 'Boolean',
'author_email' => 'String',
'author_name'  => 'String',
'created_at'   => 'Date'
}.freeze

add_field('comments', ColumnSchema.new(
  column_type: [COMMENT_THREAD_SCHEMA],
  filter_operators: []
))

# In Ticket#list:
def list(caller, filter, projection)
   records = fetch_records(filter, timezone_for(caller))
   rows = build_rows(records, projection, caller)
   rows = embed_comments(records, rows) if want_comments?(projection)
   rows
end

def want_comments?(projection)
  projection.nil? || Array(projection).map(&:to_s).any? { |p| p == 'comments' || p.start_with?('comments:') }
end

The lazy gating mirrors the existing needs_requester_email? pattern, so list views (where projection is always explicit) don't trigger N+1.

What disappears

• comment.rb (the entire Comment collection)
• The synthetic <comment_id>-<ticket_id> PK and the forest_admin_rails 1.26.2 workaround comment
• extract_field_lookup, the Branch traversal, decoded_synthetic_pairs, decode_synthetic_id
• The OneToManySchema comments relation on Ticket (replaced by the structured field)
• The log + return [] block this comment thread is anchored on
• comment_spec.rb in its entirety, replaced by smaller projection-gated tests on ticket_spec.rb

Trade-off
author_id becomes a flat author_email / author_name denormalized in the embedded structure, instead of a clickable FK to ZendeskUser. For a v1 read-only thread view it's acceptable: users read the conversation on the ticket page and don't typically jump to the agent's profile from a comment. If that need surfaces, we add it in a follow-up by reintroducing the relation.

[matthv]

The regex /[\s"]/ only triggers quoting on whitespace or double quotes. Zendesk Search has other characters that change query semantics: ( ) for grouping, : for field separators, - for negation. A value like (test gets emitted bare and Zendesk parses subject:(test as a grouping expression instead of a literal match.

[matthv]

Same query builder lives in two places: compose_full_query(caller, filter) here and Ticket#build_query(filter, timezone) in ticket.rb:119. Bodies are identical apart from how they get the timezone (mixin reads timezone_for(caller) from BaseCollection, Ticket receives it as an arg).

Worth lifting into BaseCollection#build_zendesk_query(caller, filter) and having both Searchable and Ticket call it. Single source of truth for the translator + filter.search composition, and one less place for them to drift apart.

Sketch:

# base_collection.rb
def build_zendesk_query(caller, filter)
   translated = ForestAdminDatasourceZendesk::Query::ConditionTreeTranslator.call(
    filter.condition_tree,
    timezone: timezone_for(caller),
    custom_fields: datasource.custom_field_mapping
  )
 [translated, filter.search].compact.reject(&:empty?).join(' ')
 end

# searchable.rb
def compose_full_query(caller, filter)
   build_zendesk_query(caller, filter)
 end

# ticket.rb (drop the local build_query, update its callers)
def aggregate_count(caller, filter)
  datasource.client.count('ticket', query: build_zendesk_query(caller, filter))
end

def fetch_records(filter, caller)  # signature update: pass caller, drop timezone
  ids = extract_id_lookup(filter.condition_tree)
  return ids.filter_map { |id| datasource.client.find_ticket(id) } if ids

  query = build_zendesk_query(caller, filter)
  ...
end

Once factored, compose_full_query is just a thin alias that the mixin can keep for clarity, or Searchable can call build_zendesk_query directly and the alias goes away.

[matthv]

Zendesk Search caps total results at 1000 (in addition to the per-page max of 100, which is already handled via Client::MAX_PER_PAGE). The total cap isn't represented anywhere in the code: translate_page happily computes page_num = 11 past offset 1000, sends it to Zendesk, gets a 422 back, and the user sees a generic APIError with no actionable message.
Suggested fix: add a MAX_TOTAL_RESULTS = 1000 constant, clamp page_num to MAX_TOTAL_RESULTS / per_page, and raise a ForestException with a clear message when exceeded:

MAX_TOTAL_RESULTS = 1000

[qltysh]

Function with many parameters (count = 4): put_resource [qlty:function-parameters]

[macroscopeapp]

🟢 Low forest_admin_datasource_zendesk/datasource.rb:33

When a user or organization custom field has the same column_name as a ticket custom field, build_custom_field_mapping silently drops the user/org mapping. Line 35 unconditionally writes ticket mappings, then line 39 uses ||= for user/org fields — so collisions keep the ticket value and lose the user/org zendesk_key. This causes search queries on those user/org fields to use the wrong Zendesk search field name, returning incorrect results or no matches.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb around line 33:

When a user or organization custom field has the same `column_name` as a ticket custom field, `build_custom_field_mapping` silently drops the user/org mapping. Line 35 unconditionally writes ticket mappings, then line 39 uses `||=` for user/org fields — so collisions keep the ticket value and lose the user/org `zendesk_key`. This causes search queries on those user/org fields to use the wrong Zendesk search field name, returning incorrect results or no matches.

Evidence trail:
packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/datasource.rb (lines 33-42) at REVIEWED_COMMIT: build_custom_field_mapping uses = for tickets and ||= for user/org fields.
packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/schema/custom_fields_introspector.rb (lines 73-78) at REVIEWED_COMMIT: column_naming shows ticket column_name = 'custom_<id>', user/org column_name = raw['key'] || 'custom_<id>'.
packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb:87 at REVIEWED_COMMIT: shared custom_field_mapping passed to ConditionTreeTranslator for all collection types.
packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/query/condition_tree_translator.rb:84 at REVIEWED_COMMIT: mapped_field uses @custom_fields[field] || field to resolve field names.

[macroscopeapp]

🟢 Low collections/base_collection.rb:34

When node.operator is EQUAL and node.value is nil, extract_id_lookup returns [nil]. Callers using this to short-circuit to /resource/{id} lookups attempt to fetch /resource/ with a nil ID, causing API errors. Consider returning nil when node.value is nil to skip the short-circuit.

        case node.operator
-        when Operators::EQUAL then [node.value]
+        when Operators::EQUAL then node.value.nil? ? nil : [node.value]
         when Operators::IN    then Array(node.value)

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/collections/base_collection.rb around lines 34-36:

When `node.operator` is `EQUAL` and `node.value` is `nil`, `extract_id_lookup` returns `[nil]`. Callers using this to short-circuit to `/resource/{id}` lookups attempt to fetch `/resource/` with a nil ID, causing API errors. Consider returning `nil` when `node.value` is nil to skip the short-circuit.

Evidence trail:
base_collection.rb:34 — `when Operators::EQUAL then [node.value]` returns `[nil]` when value is nil. searchable.rb:7 — `ids_in_filter(filter) ? find_records_by_id(filter) : search_records(...)` treats `[nil]` as truthy. searchable.rb:24 — `find_records_by_id` calls `find_one(nil)`. ticket.rb:104-106 — calls `fetch_tickets_by_ids([nil])`. condition_tree_leaf.rb:15 — `def initialize(field, operator, value = nil)` accepts nil value with no validation preventing EQUAL+nil combination.

[macroscopeapp]

🔴 Critical forest_admin_datasource_zendesk/forest_admin_datasource_zendesk.gemspec:32

The gemspec is missing a runtime dependency on forest_admin_datasource_toolkit, which the library requires at runtime via require 'forest_admin_datasource_toolkit' and inherits from heavily. When the gem is installed by an end user, forest_admin_datasource_toolkit will not be pulled in, causing a LoadError on startup. Add spec.add_dependency 'forest_admin_datasource_toolkit' to the gemspec.

  spec.add_dependency 'activesupport', '>= 6.1'
  spec.add_dependency 'zeitwerk', '~> 2.3'
  spec.add_dependency 'zendesk_api', '~> 3.0'
+  spec.add_dependency 'forest_admin_datasource_toolkit'

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/forest_admin_datasource_zendesk/forest_admin_datasource_zendesk.gemspec around lines 32-35:

The gemspec is missing a runtime dependency on `forest_admin_datasource_toolkit`, which the library requires at runtime via `require 'forest_admin_datasource_toolkit'` and inherits from heavily. When the gem is installed by an end user, `forest_admin_datasource_toolkit` will not be pulled in, causing a `LoadError` on startup. Add `spec.add_dependency 'forest_admin_datasource_toolkit'` to the gemspec.

[macroscopeapp]

🟢 Low query/condition_tree_translator.rb:115

In format_string, backslashes are not escaped before escaping double quotes. A value like foo\"bar becomes "foo\\"bar" after wrapping in quotes, which contains an unescaped quote and produces a malformed Zendesk query. Escape backslashes first, then double quotes.

      def format_string(value)
        return value unless value.match?(/[\s"():-]/)

-        %{"#{value.gsub('"', '\"')}"}
+        %{"#{value.gsub('\\', '\\\\\\\\').gsub('"', '\"')}"}
      end

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/query/condition_tree_translator.rb around lines 115-119:

In `format_string`, backslashes are not escaped before escaping double quotes. A value like `foo\"bar` becomes `"foo\\"bar"` after wrapping in quotes, which contains an unescaped quote and produces a malformed Zendesk query. Escape backslashes first, then double quotes.

Evidence trail:
packages/forest_admin_datasource_zendesk/lib/forest_admin_datasource_zendesk/query/condition_tree_translator.rb lines 115-118 at REVIEWED_COMMIT: `format_string` method performs `value.gsub('"', '\\"')` without first escaping backslashes, producing malformed output when the input contains `\"`.

[forest-bot]

🎉 This PR is included in version 1.28.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀