Rename Source/Sink → Client/Destination and enforce single destination credential by stuartc · Pull Request #4604 · OpenFn/lightning

stuartc · 2026-04-07T14:26:40Z

Description

Standardises channel terminology from Source/Sink to Client/Destination and
enforces that each channel has at most one destination credential (previously
unbounded via has_many).

Closes #4581, closes #4582, closes #4583

What changed

Schema rename — sink_url → destination_url, :source/:sink roles → :client/:destination, SinkAuth → DestinationAuth, event type sink_response → destination_response
has_many → has_one — sink_auth_methods (has_many) replaced by destination_auth_method (has_one, on_replace: :delete) with a partial unique DB index enforcing at-most-one
:through associations — client_webhook_auth_methods and destination_credential simplify proxy plug and form code
Form UI — Destination credential changed from checkboxes to a dropdown; field reorder to put destination URL + credential together; sublabels and "Add New" links open in new tabs
Audit trail — Moved auth method audit logic into Channels.Audit with new auth_method_changed event for destination credential swaps
Migration — Reversible column renames, enum value updates, and partial unique index
Test restructure — Split oversized channels_test.exs into channel_audit_test.exs, channel_requests_test.exs, channel_stats_test.exs; added dedicated form_test.exs
Dead code removal — list_channel_auth_methods/1, channel_proxy_path/1, supported_schemas/0

Validation steps

Run mix test test/lightning/channels_test.exs test/lightning/channels/channel_audit_test.exs test/lightning/channels/channel_requests_test.exs test/lightning/channels/channel_stats_test.exs test/lightning/channels/channel_auth_method_test.exs test/lightning/channels/destination_auth_test.exs test/lightning/channels/handler_test.exs test/lightning_web/live/channel_live/form_test.exs test/lightning_web/live/channel_live/index_test.exs test/lightning_web/plugs/channel_proxy_plug_test.exs — all pass
Create a channel with a destination credential via the form — verify dropdown works, credential saves
Edit a channel — change destination credential, verify swap; select "None", verify removal
Verify no remaining "source" or "sink" terminology in the channel UI
Run mix ecto.migrate and mix ecto.rollback --step 1 — verify migration is reversible

Additional notes for the reviewer

The migration renames columns and updates enum string values inline — no new columns or tables. Rollback is fully reversible via bidirectional execute statements.
Benchmarking suite under benchmarking/channels/ also renamed for consistency.

AI Usage

I have used Claude Code
I have used another model
I have not used AI

Pre-submission checklist

I have performed an AI review of my code (we recommend using /review with Claude Code)
I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
I have updated the changelog.
I have ticked a box in "AI usage" in this PR

#4583) Failing test contracts that define the target interface: - Rename sink_url → destination_url, source/sink roles → client/destination - Split channel_auth_methods into has_many :client_auth_methods + has_one :destination_auth_method - destination_credential through association, unique index, auto-set role via cast_assoc with: - Credential swap emits single "auth_method_changed" audit event - Update factories to match new terminology

#4581, #4582, #4583) Phase 1b + Phase 2 of the channel rename spec: - Migration: rename sink_url → destination_url, source/sink → client/destination role enum values, add destination-unique partial index, migrate event types - Schema: has_one :destination_auth_method (on_replace: :delete), has_many :client_auth_methods, has_one :destination_credential through association - Changeset: split cast_assoc with role-injecting :with callbacks - Audit: extract auth method audit helpers into Channels.Audit module - Rename SinkAuth → DestinationAuth module - Rename all sink/source references across proxy plug, handler, LiveView, form component, benchmarking, and tests

…4582, #4583)

…#4581, #4582, #4583) Replace destination credential checkboxes with a single-value <select> dropdown (None + project credentials). Reorder form fields to Name → Destination URL → Destination Credential → Enabled → Client Credentials. Add sublabels, placeholder, empty state for client credentials, and open "Add New" links in a new tab. Update build_destination_auth_param to read destination_credential_id (single value) and handle deletion of existing auth method when "None" is selected. Add :delete action clause to audit_destination_changes.

…_param (#4581, #4582, #4583)

…4583) Delete three unused public functions (channel_proxy_path/1, DestinationAuth.supported_schemas/0, list_channel_auth_methods/1) and their tests. Refactor get_channel/get_channel! to share a common query with preload support, eliminating the get-then-preload round trip.

…ks (#4581, #4582, #4583) Extract shared auth param building into build_auth_params/3 in FormComponent. Merge two identical permission-check functions into authorize_action/2 with guarded action atoms in Index.

…sts (#4581, #4582, #4583) Split channels_test.exs (1390 → 403 lines) into focused files: channel_requests_test, channel_stats_test, channel_audit_test. Split index_test.exs (865 → 296 lines) by extracting form_test. Merged micro-tests with shared setup to reduce 62 → 40 context tests without losing assertion coverage.

…ame (#4581, #4582, #4583) Deduplicate source_event_path/1 and error_event_message/1 in channel_logs_component via shared get_event_field/3 helper. Remove stale 'sink_url' from audit test description while keeping the transitional refute assertion.

…4583) Rename MockSink → MockDestination, --sink → --destination flag, slow_sink/direct_sink → slow_destination/direct_destination scenarios, and all references across config, setup, runner, report, README, and run_all.sh. File renamed mock_sink.exs → mock_destination.exs.

…queries (#4581, #4582, #4583) Add has_many :client_webhook_auth_methods through association to Channel schema (symmetric with existing destination_credential). Include the :through binding in get_channel_with_auth join-preload so it resolves in a single SQL query. Simplify proxy plug to use the :through accessor directly. Refactor get_channel_for_project to build on the shared channel_query/2 with :include support.

…4581, #4582, #4583) Derive extract_fields and wrap functions once from the direction atom instead of switching on it per iteration inside Enum.reduce.

…hod (#4581, #4582, #4583) Replace the has_many-style delete: true virtual field pattern with Ecto's native on_replace: :delete nil path for the has_one destination_auth_method. Also harden has_string_keys? against empty maps with a guard clause.

…redential (#4581, #4582, #4583)

codecov · 2026-04-07T14:34:51Z

Codecov Report

❌ Patch coverage is 92.96875% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.52%. Comparing base (bf0687f) to head (e1a3cab).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/lightning/channels/audit.ex	89.74%	4 Missing ⚠️
...htning_web/live/run_live/channel_logs_component.ex	50.00%	2 Missing ⚠️
lib/lightning/channels.ex	85.71%	1 Missing ⚠️
lib/lightning/channels/channel.ex	85.71%	1 Missing ⚠️
.../lightning_web/live/channel_live/form_component.ex	98.07%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4604      +/-   ##
==========================================
+ Coverage   89.50%   89.52%   +0.02%     
==========================================
  Files         441      442       +1     
  Lines       21295    21319      +24     
==========================================
+ Hits        19060    19086      +26     
+ Misses       2235     2233       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

elias-ba

Hey @stuartc, huge and really nice piece of work here 👏🏽 . Everything looks good to me. I've left a few questions/comments on things that caught my attention, but nothing blocking. Up to you whether to address them.

elias-ba · 2026-04-08T21:00:47Z

lib/lightning/channels/channel.ex

+  end
+
+  defp has_string_keys?(attrs) when map_size(attrs) == 0, do: false
+  defp has_string_keys?(attrs), do: attrs |> Map.keys() |> hd() |> is_binary()


Since map key ordering isn't guaranteed, could hd() give inconsistent results for maps that happen to have mixed key types? Would something like Enum.any?(&is_binary/1) be safer here, or is there a guarantee upstream that keys are always uniform?

As discussed, we're still protected by Ectos changeset check, so I thing this is find for now.

elias-ba · 2026-04-08T21:03:22Z

lib/lightning_web/live/channel_live/index.ex

+          include: [
+            :channel_auth_methods,
+            :client_auth_methods,
+            :destination_auth_method
+          ]


Just wondering, since :client_auth_methods and :destination_auth_method are both subsets of :channel_auth_methods on the same join table, does preloading all three result in redundant queries? Would it make sense to preload just :channel_auth_methods and filter in memory, or is the clarity worth the extra round trips here?

You're right, this is a hangover from when I only had :channel_auth_methods - it's supposed to be just the other two, and we don't use the through association to find them. So yeah, I'm still keeping the preloads, it's 2 instead of 3 queries. Not exactly that you are proposing here, but it's super cheap and since it's an on mount concern I'm opting for interface clarity.

elias-ba · 2026-04-08T21:05:24Z

test/lightning/channels/channel_audit_test.exs

+      # TODO: Remove this refutation once the old two-event pattern is fully
+      # gone — at that point added/removed won't fire for swaps and this
+      # negative assertion adds no value.


Would it make sense to open a small follow-up issue to track removing these transitional assertions once the migration has been run everywhere? Just so they don't get forgotten.

I'm in two minds, it's so small, feels weird to make an issue for it. Also I don't think it's actually testing that an actual migration has been run now that I look closer. I'll address this in the next PR.

…thods (#4581, #4582, #4583) FormComponent now reads client_auth_methods and destination_auth_method directly, removing the redundant channel_auth_methods preload and manual role filtering.

stuartc added 14 commits April 7, 2026 16:22

test: define Phase 3a form contracts for channel dropdown UI (#4581, #…

5abd9d5

…4582, #4583)

chore: replace cond with case tuple pattern in build_destination_auth…

ad67e5a

…_param (#4581, #4582, #4583)

refactor: deduplicate save_channel param assembly and permission chec…

24c09c6

…ks (#4581, #4582, #4583) Extract shared auth param building into build_auth_params/3 in FormComponent. Merge two identical permission-check functions into authorize_action/2 with guarded action atoms in Index.

refactor: hoist direction-dependent logic out of audit reduce loop (#…

3db430c

…4581, #4582, #4583) Derive extract_fields and wrap functions once from the direction atom instead of switching on it per iteration inside Enum.reduce.

docs: add changelog entry for channel rename and single destination c…

455273c

…redential (#4581, #4582, #4583)

github-project-automation bot added this to Core Apr 7, 2026

github-project-automation bot moved this to New Issues in Core Apr 7, 2026

stuartc requested a review from elias-ba April 7, 2026 14:27

stuartc self-assigned this Apr 7, 2026

elias-ba approved these changes Apr 8, 2026

View reviewed changes

elias-ba and others added 2 commits April 8, 2026 21:11

chore: fix tree diagram alignment in benchmarking README

77ce357

refactor: use typed associations instead of filtering channel_auth_me…

e1a3cab

…thods (#4581, #4582, #4583) FormComponent now reads client_auth_methods and destination_auth_method directly, removing the redundant channel_auth_methods preload and manual role filtering.

stuartc merged commit d582713 into main Apr 9, 2026
8 checks passed

stuartc deleted the 4581-4582-4583-channel-rename-destination-credential branch April 9, 2026 12:10

github-project-automation bot moved this from New Issues to Done in Core Apr 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename Source/Sink → Client/Destination and enforce single destination credential#4604

Rename Source/Sink → Client/Destination and enforce single destination credential#4604
stuartc merged 16 commits intomainfrom
4581-4582-4583-channel-rename-destination-credential

stuartc commented Apr 7, 2026

Uh oh!

codecov bot commented Apr 7, 2026 •

edited

Loading

Uh oh!

elias-ba left a comment

Uh oh!

elias-ba Apr 8, 2026

Uh oh!

stuartc Apr 9, 2026

Uh oh!

elias-ba Apr 8, 2026

Uh oh!

stuartc Apr 9, 2026

Uh oh!

elias-ba Apr 8, 2026

Uh oh!

stuartc Apr 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

stuartc commented Apr 7, 2026

Description

What changed

Validation steps

Additional notes for the reviewer

AI Usage

Pre-submission checklist

Uh oh!

codecov bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

elias-ba left a comment

Choose a reason for hiding this comment

Uh oh!

elias-ba Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

stuartc Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

elias-ba Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

stuartc Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

elias-ba Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

stuartc Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Apr 7, 2026 •

edited

Loading