From 643a2f742401e0b877f45556115274af94a6dd67 Mon Sep 17 00:00:00 2001 From: Jordi Enric Date: Fri, 17 Apr 2026 09:52:04 +0200 Subject: [PATCH] =?UTF-8?q?refactor(log-drains):=20simplify=20update=20?= =?UTF-8?q?=E2=80=94=20send=20full=20form=20state,=20strip=20REDACTED=20he?= =?UTF-8?q?aders?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove computePatchPayload diff logic. Just send the full form state to the PATCH endpoint with REDACTED header sentinels stripped. The backend is responsible for merging with stored values. Co-Authored-By: Claude Sonnet 4.6 --- .../LogDrains/LogDrains.utils.test.ts | 138 ------------------ .../interfaces/LogDrains/LogDrains.utils.ts | 65 --------- .../log-drains/update-log-drain-mutation.ts | 19 ++- .../project/[ref]/settings/log-drains.tsx | 47 ++---- 4 files changed, 21 insertions(+), 248 deletions(-) diff --git a/apps/studio/components/interfaces/LogDrains/LogDrains.utils.test.ts b/apps/studio/components/interfaces/LogDrains/LogDrains.utils.test.ts index d589fa83bc3..fd45807874e 100644 --- a/apps/studio/components/interfaces/LogDrains/LogDrains.utils.test.ts +++ b/apps/studio/components/interfaces/LogDrains/LogDrains.utils.test.ts @@ -2,7 +2,6 @@ import { describe, expect, it } from 'vitest' import { computePatchHeaders, - computePatchPayload, getDefaultHeadersByType, getHeadersSectionDescription, HEADER_VALIDATION_ERRORS, @@ -393,140 +392,3 @@ describe('computePatchHeaders', () => { expect(computePatchHeaders({ 'X-Info': 'redacted' })).toEqual({ 'X-Info': 'redacted' }) }) }) - -describe('computePatchPayload', () => { - const baseOriginal = { - name: 'My Drain', - description: 'A description', - type: 'webhook' as const, - config: { url: 'https://example.com', http: 'http2', gzip: true } as Record, - } - - it('returns empty object when nothing changed', () => { - expect( - computePatchPayload( - { - name: 'My Drain', - description: 'A description', - type: 'webhook', - config: { url: 'https://example.com', http: 'http2', gzip: true }, - }, - baseOriginal - ) - ).toEqual({}) - }) - - it('includes name when changed', () => { - expect(computePatchPayload({ ...baseOriginal, name: 'New Name' }, baseOriginal)).toEqual({ - name: 'New Name', - }) - }) - - it('includes description when changed', () => { - expect(computePatchPayload({ ...baseOriginal, description: 'Updated' }, baseOriginal)).toEqual({ - description: 'Updated', - }) - }) - - it('never includes type even if different', () => { - const result = computePatchPayload({ ...baseOriginal, type: 'loki' as const }, baseOriginal) - expect(result).not.toHaveProperty('type') - }) - - it('includes only the changed config field', () => { - expect( - computePatchPayload( - { ...baseOriginal, config: { url: 'https://new-url.com', http: 'http2', gzip: true } }, - baseOriginal - ) - ).toEqual({ config: { url: 'https://new-url.com' } }) - }) - - it('includes multiple changed config fields', () => { - expect( - computePatchPayload( - { ...baseOriginal, config: { url: 'https://new-url.com', http: 'http1', gzip: true } }, - baseOriginal - ) - ).toEqual({ config: { url: 'https://new-url.com', http: 'http1' } }) - }) - - it('detects boolean toggle in config', () => { - expect( - computePatchPayload( - { ...baseOriginal, config: { url: 'https://example.com', http: 'http2', gzip: false } }, - baseOriginal - ) - ).toEqual({ config: { gzip: false } }) - }) - - it('skips headers when undefined (all-REDACTED sentinel — nothing changed)', () => { - const original = { - ...baseOriginal, - config: { ...baseOriginal.config, headers: { Authorization: 'secret' } }, - } - expect( - computePatchPayload( - { ...baseOriginal, config: { ...baseOriginal.config, headers: undefined } }, - original - ) - ).toEqual({}) - }) - - it('includes headers: {} when user cleared all headers', () => { - const original = { - ...baseOriginal, - config: { ...baseOriginal.config, headers: { Authorization: 'secret' } }, - } - expect( - computePatchPayload( - { ...baseOriginal, config: { ...baseOriginal.config, headers: {} } }, - original - ) - ).toEqual({ config: { headers: {} } }) - }) - - it('includes new headers when user added them', () => { - const original = { - ...baseOriginal, - config: { ...baseOriginal.config, headers: { Authorization: 'secret' } }, - } - expect( - computePatchPayload( - { ...baseOriginal, config: { ...baseOriginal.config, headers: { 'X-New': 'value' } } }, - original - ) - ).toEqual({ config: { headers: { 'X-New': 'value' } } }) - }) - - it('includes both top-level and config changes', () => { - expect( - computePatchPayload( - { - name: 'Renamed', - description: 'A description', - type: 'webhook', - config: { url: 'https://new.com', http: 'http2', gzip: false }, - }, - baseOriginal - ) - ).toEqual({ name: 'Renamed', config: { url: 'https://new.com', gzip: false } }) - }) - - it('treats undefined and empty string description as equivalent', () => { - const original = { ...baseOriginal, description: undefined } - expect(computePatchPayload({ ...baseOriginal, description: '' }, original)).toEqual({}) - }) - - it('works with non-webhook config shapes (e.g. datadog)', () => { - const original = { - name: 'DD', - description: '', - type: 'datadog' as const, - config: { api_key: 'key123', region: 'us1' } as Record, - } - expect( - computePatchPayload({ ...original, config: { api_key: 'key123', region: 'eu1' } }, original) - ).toEqual({ config: { region: 'eu1' } }) - }) -}) diff --git a/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts b/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts index f7a06aa3b72..5d28ff1f381 100644 --- a/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts +++ b/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts @@ -100,71 +100,6 @@ export function computePatchHeaders( return Object.fromEntries(changed) } -type LogDrainPatchValues = { - name?: string - description?: string - config?: Record -} - -/** - * Computes a minimal PATCH payload by diffing submitted form values against the - * original log drain data. Only includes fields that actually changed. - * - * - `type` is always omitted — it cannot change on update. - * - `headers: undefined` in config means "all REDACTED, nothing changed" — skipped. - * - `headers: {}` means "user cleared all headers" — included. - * - Other config fields are compared by strict equality (primitives) or JSON (objects). - * - Returns `{}` when nothing changed — the caller should skip the API call. - */ -export function computePatchPayload( - submitted: { - name: string - description?: string - type: LogDrainType - config: Record - }, - original: { - name: string - description?: string - type: LogDrainType - config: Record - } -): LogDrainPatchValues { - const patchPayload: LogDrainPatchValues = {} - - if (submitted.name !== original.name) { - patchPayload.name = submitted.name - } - - if ((submitted.description ?? '') !== (original.description ?? '')) { - patchPayload.description = submitted.description ?? '' - } - - const configPatch: Record = {} - for (const key of Object.keys(submitted.config)) { - const submittedVal = submitted.config[key] - const originalVal = original.config[key] - - // headers: undefined means computePatchHeaders determined nothing changed — skip - if (key === 'headers' && submittedVal === undefined) continue - - const isEqual = - typeof submittedVal === 'object' && submittedVal !== null - ? JSON.stringify(submittedVal) === JSON.stringify(originalVal) - : submittedVal === originalVal - - if (!isEqual) { - configPatch[key] = submittedVal - } - } - - if (Object.keys(configPatch).length > 0) { - patchPayload.config = configPatch - } - - return patchPayload -} - export const logDrainHeaderEntriesSchema = z .array( z.object({ diff --git a/apps/studio/data/log-drains/update-log-drain-mutation.ts b/apps/studio/data/log-drains/update-log-drain-mutation.ts index 940576cb44a..d39a39b5637 100644 --- a/apps/studio/data/log-drains/update-log-drain-mutation.ts +++ b/apps/studio/data/log-drains/update-log-drain-mutation.ts @@ -10,10 +10,10 @@ export type LogDrainUpdateVariables = { projectRef: string token?: string id?: string | number - name?: string + name: string description?: string - type?: LogDrainType - config?: Record + type: LogDrainType + config: Record } export async function updateLogDrain(payload: LogDrainUpdateVariables) { @@ -21,15 +21,14 @@ export async function updateLogDrain(payload: LogDrainUpdateVariables) { throw new Error('Token is required') } - const body: Record = {} - if (payload.name !== undefined) body.name = payload.name - if (payload.description !== undefined) body.description = payload.description - if (payload.type !== undefined) body.type = payload.type - if (payload.config !== undefined) body.config = payload.config - const { data, error } = await patch('/platform/projects/{ref}/analytics/log-drains/{token}', { params: { path: { ref: payload.projectRef, token: payload.token } }, - body: body as any, + body: { + name: payload.name, + description: payload.description, + type: payload.type, + config: payload.config as any, + }, }) if (error) handleError(error) diff --git a/apps/studio/pages/project/[ref]/settings/log-drains.tsx b/apps/studio/pages/project/[ref]/settings/log-drains.tsx index aec66a1d7e2..791e64669e5 100644 --- a/apps/studio/pages/project/[ref]/settings/log-drains.tsx +++ b/apps/studio/pages/project/[ref]/settings/log-drains.tsx @@ -20,7 +20,6 @@ import { LOG_DRAIN_TYPES, LogDrainType, } from '@/components/interfaces/LogDrains/LogDrains.constants' -import { computePatchPayload } from '@/components/interfaces/LogDrains/LogDrains.utils' import DefaultLayout from '@/components/layouts/DefaultLayout' import { PageLayout } from '@/components/layouts/PageLayout/PageLayout' import SettingsLayout from '@/components/layouts/ProjectSettingsLayout/SettingsLayout' @@ -121,46 +120,24 @@ const LogDrainsSettings: NextPageWithLayout = () => { }} isLoading={isLoading} onSubmit={({ name, description, type, ...values }) => { + const logDrainValues = { + name, + description: description || '', + type, + config: values as any, // TODO: fix generated API types from backend + id: selectedLogDrain?.id, + projectRef: ref, + token: selectedLogDrain?.token, + } + if (mode === 'create') { - const logDrainValues = { - name, - description: description || '', - type, - config: values as any, // TODO: fix generated API types from backend - id: selectedLogDrain?.id, - projectRef: ref, - token: selectedLogDrain?.token, - } setPendingLogDrainValues(logDrainValues) setIsCreateConfirmModalOpen(true) } else { - if (!selectedLogDrain?.id || !selectedLogDrain?.token) { + if (!logDrainValues.id || !selectedLogDrain?.token) { throw new Error('Log drain ID and token is required') } - - const patchPayload = computePatchPayload( - { name, description, type, config: values as Record }, - { - name: selectedLogDrain.name!, - description: selectedLogDrain.description, - type: selectedLogDrain.type!, - config: (selectedLogDrain.config ?? {}) as Record, - } - ) - - if (Object.keys(patchPayload).length === 0) { - toast.info('No changes to save') - setOpen(false) - return - } - - updateLogDrain({ - ...patchPayload, - type, // required by the platform endpoint even for partial updates - projectRef: ref, - token: selectedLogDrain.token, - id: selectedLogDrain.id, - }) + updateLogDrain(logDrainValues) } }} />