mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 08:01:35 +00:00
Revert "fix: stop counting empty schema-added fields and server metadata as drafts"
This reverts commit 9787270ad8.
This commit is contained in:
@@ -200,15 +200,13 @@ function snapshotDraftValue<V>(value: V | undefined): V | undefined {
|
||||
*
|
||||
* The rest are server-managed read-time metadata that ride along on the
|
||||
* loaded deployed payload but never appear in the editor's draft content, so
|
||||
* comparing them would mask a true baseline match: `draft_saved_at` (the
|
||||
* draft's own save time), `edited_at` (deploy time), `edited_by` (deploy
|
||||
* author), `workspace_id`, `version_id` (deployed version), `is_draft`
|
||||
* (backend presence flag), `assets` (server-derived on deploy from the
|
||||
* content — the editor's draft value never carries it, so a deployed
|
||||
* baseline's `assets: []` would otherwise never equal the assets-less draft
|
||||
* and every untouched load would autosave a phantom draft), and the groups
|
||||
* below. A field belongs here only if no editor lets anyone change it —
|
||||
* listing an editable one makes a real edit invisible.
|
||||
* comparing them would mask a true baseline match:
|
||||
* `draft_saved_at` (the draft's own save time), `edited_at` (deploy time),
|
||||
* `edited_by` (deploy author), `workspace_id`, `version_id` (deployed version),
|
||||
* `is_draft` (backend presence flag), and `assets` (server-derived on deploy
|
||||
* from the content — the editor's draft value never carries it, so a deployed
|
||||
* baseline's `assets: []` would otherwise never equal the assets-less draft and
|
||||
* every untouched load would autosave a phantom draft).
|
||||
*/
|
||||
const DRAFT_COMPARE_IGNORED_FIELDS = [
|
||||
'permissioned_as',
|
||||
@@ -222,76 +220,16 @@ const DRAFT_COMPARE_IGNORED_FIELDS = [
|
||||
'parent_version',
|
||||
'is_draft',
|
||||
'assets',
|
||||
// Authorship and deploy identity, assigned by the server.
|
||||
'created_by',
|
||||
'created_at',
|
||||
'hash',
|
||||
'parent_hashes',
|
||||
'lock_error_logs',
|
||||
// Presence and lifecycle flags the backend computes per read.
|
||||
'draft_only',
|
||||
'no_deployed',
|
||||
'starred',
|
||||
'archived',
|
||||
'deleted',
|
||||
// Computed at read time from the parent folder; edited on the folder, not
|
||||
// here. `labels` itself IS editable and stays compared.
|
||||
'inherited_labels',
|
||||
// Fixed at creation and absent from the resource editor's draft shape, so
|
||||
// it only ever shows up on the deployed side of a comparison.
|
||||
'resource_type',
|
||||
// A resource's OAuth/linked-secret state: owned by the OAuth flow and the
|
||||
// variable it points at, never by the resource form.
|
||||
'is_oauth',
|
||||
'is_linked',
|
||||
'is_refreshed',
|
||||
'is_expired',
|
||||
'refresh_error',
|
||||
'account',
|
||||
// A schedule's last-trigger error and its owner's email, both written by
|
||||
// the scheduler rather than the editor.
|
||||
'error',
|
||||
'email'
|
||||
'resource_type'
|
||||
] as const
|
||||
|
||||
/**
|
||||
* Whether a value means "nothing is set here". A key holding one is dropped
|
||||
* below, so a key that only exists because a schema gained a property — the
|
||||
* form materializes `''`, `[]`, `{}` or `false` into every item that never
|
||||
* carried it — compares equal to no key at all.
|
||||
*
|
||||
* `0` is deliberately NOT empty: nothing materializes it, and it is a value
|
||||
* someone picks (a timeout, a retry count). `false` is, because the only way
|
||||
* an unchecked box differs from an absent key is that a form drew it.
|
||||
*/
|
||||
function isEmptyDraftValue(v: unknown): boolean {
|
||||
if (v === undefined || v === null || v === '' || v === false) return true
|
||||
if (Array.isArray(v)) return v.length === 0
|
||||
return typeof v === 'object' && Object.keys(v as object).length === 0
|
||||
}
|
||||
|
||||
/** Drop empty-valued keys at every depth, innermost first so a branch that
|
||||
* empties out goes with them. Mutates — only ever called on a fresh parse. */
|
||||
function dropEmptyKeys(v: unknown): void {
|
||||
if (Array.isArray(v)) {
|
||||
// Never drop elements: that would shift every index after them.
|
||||
for (const e of v) dropEmptyKeys(e)
|
||||
return
|
||||
}
|
||||
if (v === null || typeof v !== 'object') return
|
||||
for (const [k, child] of Object.entries(v as Record<string, unknown>)) {
|
||||
dropEmptyKeys(child)
|
||||
if (isEmptyDraftValue(child)) delete (v as Record<string, unknown>)[k]
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize one side of a draft-vs-baseline comparison: JSON round-trip
|
||||
* (drafts are stored as json server-side, which strips `undefined` keys,
|
||||
* so `{ labels: undefined }` and `{}` must compare equal), drop the ignored
|
||||
* fields above — top level only, so an item's own content may still use those
|
||||
* names — and drop empty-valued keys at every depth. Returns the input
|
||||
* unchanged if unserializable.
|
||||
* so `{ labels: undefined }` and `{}` must compare equal) and drop the
|
||||
* ignored fields above. Returns the input unchanged if unserializable.
|
||||
*/
|
||||
export function normalizeDraftForCompare<V>(value: V): V {
|
||||
try {
|
||||
@@ -299,7 +237,6 @@ export function normalizeDraftForCompare<V>(value: V): V {
|
||||
if (v !== null && typeof v === 'object' && !Array.isArray(v)) {
|
||||
for (const f of DRAFT_COMPARE_IGNORED_FIELDS) delete v[f]
|
||||
}
|
||||
dropEmptyKeys(v)
|
||||
return v as V
|
||||
} catch {
|
||||
return value
|
||||
|
||||
@@ -1,63 +0,0 @@
|
||||
import { describe, it, expect, vi } from 'vitest'
|
||||
|
||||
// Importing the module pulls in the generated client and the syncer's
|
||||
// localStorage / pagehide wiring; stub the same surfaces the syncer tests do
|
||||
// so the pure-function import stays side-effect free.
|
||||
vi.mock('./gen', () => ({ DraftService: { updateDraft: vi.fn() } }))
|
||||
vi.mock('./gen/core/OpenAPI', () => ({ OpenAPI: { BASE: '' } }))
|
||||
vi.mock('./userDraftDbSyncer.svelte', () => ({ UserDraftDbSyncer: { save: vi.fn() } }))
|
||||
|
||||
import { draftValuesEqual } from './userDraft.svelte'
|
||||
|
||||
describe('draftValuesEqual', () => {
|
||||
it('ignores a key the schema added but nobody filled in', () => {
|
||||
const deployed = { args: { host: 'example.com' } }
|
||||
const settled = { args: { host: 'example.com', port: '', ssl: false, tags: [] } }
|
||||
expect(draftValuesEqual(settled, deployed)).toBe(true)
|
||||
})
|
||||
|
||||
it('still sees the same key once it holds something', () => {
|
||||
const deployed = { args: { host: 'example.com' } }
|
||||
expect(draftValuesEqual({ args: { host: 'example.com', port: '5432' } }, deployed)).toBe(false)
|
||||
})
|
||||
|
||||
it('sees a value being cleared, because the baseline had one', () => {
|
||||
expect(draftValuesEqual({ args: { host: '' } }, { args: { host: 'example.com' } })).toBe(false)
|
||||
})
|
||||
|
||||
it('treats 0 as a real value, not as empty', () => {
|
||||
expect(draftValuesEqual({ timeout: 0 }, {})).toBe(false)
|
||||
})
|
||||
|
||||
it('drops a nested branch that empties out entirely', () => {
|
||||
expect(draftValuesEqual({ args: { auth: { token: '' } } }, {})).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps array positions when an element empties out', () => {
|
||||
expect(draftValuesEqual({ xs: [{ a: '' }, { a: 'x' }] }, { xs: [{}, { a: 'x' }] })).toBe(true)
|
||||
expect(draftValuesEqual({ xs: [{ a: 'x' }, { a: '' }] }, { xs: [{}, { a: 'x' }] })).toBe(false)
|
||||
})
|
||||
|
||||
it('ignores server-managed metadata riding along on the deployed payload', () => {
|
||||
const deployed = {
|
||||
path: 'u/me/r',
|
||||
value: { host: 'h' },
|
||||
created_by: 'admin',
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
is_oauth: false,
|
||||
inherited_labels: ['from-folder'],
|
||||
starred: true
|
||||
}
|
||||
expect(draftValuesEqual({ path: 'u/me/r', value: { host: 'h' } }, deployed)).toBe(true)
|
||||
})
|
||||
|
||||
it('still compares labels, which are edited here', () => {
|
||||
expect(draftValuesEqual({ labels: ['a'] }, { labels: ['b'] })).toBe(false)
|
||||
})
|
||||
|
||||
it('only ignores metadata names at the top level', () => {
|
||||
const a = { value: { created_by: 'someone' } }
|
||||
const b = { value: { created_by: 'someone else' } }
|
||||
expect(draftValuesEqual(a, b)).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -60,15 +60,6 @@ describe('pruneMeaninglessDrafts', () => {
|
||||
expect(discardDraft).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('ignores the empty fields a moved-on schema added', async () => {
|
||||
listDrafts.mockResolvedValue([row()])
|
||||
getDraftDiffValues.mockResolvedValue(
|
||||
diff({ draft: { value: { host: 'h', port: '', ssl: false, tags: [] } } })
|
||||
)
|
||||
await pruneMeaninglessDrafts('main', 'me@x.dev')
|
||||
expect(discardedPaths()).toEqual(['u/me/r'])
|
||||
})
|
||||
|
||||
it('never touches a draft-only item — the draft is the whole item', async () => {
|
||||
listDrafts.mockResolvedValue([row({ draft_only: true })])
|
||||
getDraftDiffValues.mockResolvedValue(diff())
|
||||
|
||||
Reference in New Issue
Block a user