Skip to content

Commit a073ee5

Browse files
committed
fix(org): treat a failed retention re-read as unconfirmed
refetch() resolves on failure with the last good data, so a save or removal could write a stale document over another admin's change.
1 parent ba0054d commit a073ee5

2 files changed

Lines changed: 49 additions & 3 deletions

File tree

‎apps/sim/platform/app/(shell)/settings/_components/sections/data-retention/data-retention-section.round-trip.test.tsx‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@ vi.mock(
3030
changed,
3131
onChange,
3232
onSave,
33+
onRemove,
3334
}: {
3435
draft: PolicyDraft
3536
changed: boolean
3637
onChange: (draft: PolicyDraft) => void
3738
onSave: () => void
39+
onRemove: () => Promise<void>
3840
}): ReactNode => (
3941
<div>
4042
<output>{draft.logDays}</output>
@@ -47,6 +49,16 @@ vi.mock(
4749
<button type='button' disabled={!changed} onClick={onSave}>
4850
Save
4951
</button>
52+
<button
53+
type='button'
54+
onClick={() =>
55+
void onRemove().catch((error: Error) => {
56+
window.document.body.dataset.removeError = error.message
57+
})
58+
}
59+
>
60+
Remove
61+
</button>
5062
</div>
5163
),
5264
})
@@ -122,6 +134,7 @@ describe('DataRetentionSection across saves', () => {
122134
server.writes = []
123135
server.answer = (body) => retentionDocument((body.logRetentionHours ?? 0) / 24)
124136
server.read = async () => ({ success: true, data: server.document })
137+
delete window.document.body.dataset.removeError
125138
apiClientRequestMockFns.mockRequestJson
126139
.mockReset()
127140
.mockImplementation(
@@ -198,6 +211,38 @@ describe('DataRetentionSection across saves', () => {
198211
expect(emcnMockFns.mockToast.error).not.toHaveBeenCalled()
199212
})
200213

214+
describe('the read before a write fails', () => {
215+
async function failNextReads(start: () => Promise<void>) {
216+
server.read = async () => {
217+
throw new Error('read failed')
218+
}
219+
await start()
220+
await settle()
221+
}
222+
223+
it('writes nothing over the document from before the failure, and says so', async () => {
224+
server.document = retentionDocument(60)
225+
await click('Organization')
226+
await click('keep logs 7 days')
227+
await failNextReads(() => click('Save'))
228+
229+
expect(server.writes).toEqual([])
230+
expect(emcnMockFns.mockToast.error).toHaveBeenCalledWith(
231+
'Could not confirm the latest retention settings. Nothing was saved.'
232+
)
233+
})
234+
235+
it('removes nothing over the document from before the failure', async () => {
236+
await click('Organization')
237+
await failNextReads(() => click('Remove'))
238+
239+
expect(server.writes).toEqual([])
240+
expect(window.document.body.dataset.removeError).toBe(
241+
'Could not confirm the latest retention settings.'
242+
)
243+
})
244+
})
245+
201246
it('shows the route answer without waiting on the read that follows the save', async () => {
202247
server.answer = () => retentionDocument(90)
203248
await click('Organization')

‎apps/sim/platform/app/(shell)/settings/_components/sections/data-retention/data-retention-section.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ interface RetentionFormProps {
199199
initialData: DataRetentionResponse
200200
organizationId: string
201201
workspaceOptions: { value: string; label: string }[]
202-
refetch: () => Promise<{ data?: DataRetentionResponse }>
202+
refetch: () => Promise<{ data?: DataRetentionResponse; isError: boolean }>
203203
}
204204

205205
/**
@@ -236,8 +236,9 @@ function RetentionForm({
236236
withOverrides={withOverrides}
237237
isSaving={update.isPending}
238238
readLatest={async () => {
239-
const { data } = await refetch()
240-
return data ? snapshotFrom(data) : null
239+
// A failed refetch still resolves, carrying the last good document.
240+
const { data, isError } = await refetch()
241+
return data && !isError ? snapshotFrom(data) : null
241242
}}
242243
persist={persist}
243244
onClose={() => setSelection(null)}

0 commit comments

Comments
 (0)