mirror of
https://github.com/supabase/supabase.git
synced 2026-10-10 11:55:05 +03:00
refactor(log-drains): simplify update — send full form state, strip REDACTED headers
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
135a973bc7
commit
643a2f7424
4 files changed
+21
-248
No files matched your search
@@ -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<string, unknown>,
|
||||
}
|
||||
|
||||
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<string, unknown>,
|
||||
}
|
||||
expect(
|
||||
computePatchPayload({ ...original, config: { api_key: 'key123', region: 'eu1' } }, original)
|
||||
).toEqual({ config: { region: 'eu1' } })
|
||||
})
|
||||
})
|
||||
@@ -100,71 +100,6 @@ export function computePatchHeaders(
|
||||
return Object.fromEntries(changed)
|
||||
}
|
||||
|
||||
type LogDrainPatchValues = {
|
||||
name?: string
|
||||
description?: string
|
||||
config?: Record<string, unknown>
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<string, unknown>
|
||||
},
|
||||
original: {
|
||||
name: string
|
||||
description?: string
|
||||
type: LogDrainType
|
||||
config: Record<string, unknown>
|
||||
}
|
||||
): 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<string, unknown> = {}
|
||||
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({
|
||||
|
||||
@@ -10,10 +10,10 @@ export type LogDrainUpdateVariables = {
|
||||
projectRef: string
|
||||
token?: string
|
||||
id?: string | number
|
||||
name?: string
|
||||
name: string
|
||||
description?: string
|
||||
type?: LogDrainType
|
||||
config?: Record<string, unknown>
|
||||
type: LogDrainType
|
||||
config: Record<string, unknown>
|
||||
}
|
||||
|
||||
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<string, unknown> = {}
|
||||
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)
|
||||
|
||||
@@ -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<string, unknown> },
|
||||
{
|
||||
name: selectedLogDrain.name!,
|
||||
description: selectedLogDrain.description,
|
||||
type: selectedLogDrain.type!,
|
||||
config: (selectedLogDrain.config ?? {}) as Record<string, unknown>,
|
||||
}
|
||||
)
|
||||
|
||||
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)
|
||||
}
|
||||
}}
|
||||
/>
|
||||
|
||||
Reference in new issue
Block a user