Skip to content

Commit 79d610d

Browse files
authored
Fix instance disk menus closing on poll (#2618)
fix instance disk menus closing on poll
1 parent bdbc02b commit 79d610d

File tree

3 files changed

+74
-10
lines changed

3 files changed

+74
-10
lines changed

app/pages/project/instances/instance/tabs/StorageTab.tsx

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,6 @@ export function Component() {
146146
[disks.items, instance.bootDiskId]
147147
)
148148

149-
// Needed to keep them the same while setting boot disk.
150-
// Extracted to keep dep array appropriately zealous.
151-
const { ncpus, memory } = instance
152-
153149
const makeBootDiskActions = useCallback(
154150
(disk: InstanceDisk): MenuAction[] => [
155151
getSnapshotAction(disk),
@@ -168,8 +164,8 @@ export function Component() {
168164
path: { instance: instance.id },
169165
body: {
170166
bootDisk: undefined,
171-
ncpus,
172-
memory,
167+
ncpus: instance.ncpus,
168+
memory: instance.memory,
173169
// this would get unset if we left it out
174170
autoRestartPolicy: instance.autoRestartPolicy,
175171
},
@@ -200,7 +196,16 @@ export function Component() {
200196
onActivate() {}, // it's always disabled, so noop is ok
201197
},
202198
],
203-
[instanceUpdate, instance, getSnapshotAction, ncpus, memory]
199+
[
200+
instanceUpdate,
201+
// don't put the entire instance in here. it is not referentially
202+
// stable across polls, so the menus will close during polling
203+
instance.id,
204+
instance.autoRestartPolicy,
205+
instance.ncpus,
206+
instance.memory,
207+
getSnapshotAction,
208+
]
204209
)
205210

206211
const makeOtherDiskActions = useCallback(
@@ -223,8 +228,8 @@ export function Component() {
223228
path: { instance: instance.id },
224229
body: {
225230
bootDisk: disk.id,
226-
ncpus,
227-
memory,
231+
ncpus: instance.ncpus,
232+
memory: instance.memory,
228233
// this would get unset if we left it out
229234
autoRestartPolicy: instance.autoRestartPolicy,
230235
},
@@ -262,7 +267,18 @@ export function Component() {
262267
},
263268
},
264269
],
265-
[detachDisk, instanceUpdate, instance, getSnapshotAction, bootDisks, ncpus, memory]
270+
[
271+
detachDisk,
272+
instanceUpdate,
273+
// don't put the entire instance in here. it is not referentially
274+
// stable across polls, so the menus will close during polling
275+
instance.id,
276+
instance.autoRestartPolicy,
277+
instance.ncpus,
278+
instance.memory,
279+
getSnapshotAction,
280+
bootDisks,
281+
]
266282
)
267283

268284
const attachDisk = useApiMutation('instanceDiskAttach', {

test/e2e/instance-disks.e2e.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77
*/
88
import {
99
clickRowAction,
10+
closeToast,
1011
expect,
1112
expectNoToast,
1213
expectNotVisible,
1314
expectRowVisible,
1415
expectToast,
1516
expectVisible,
17+
openRowActions,
1618
stopInstance,
1719
test,
1820
} from './utils'
@@ -224,3 +226,45 @@ test('Change boot disk', async ({ page }) => {
224226

225227
await expect(page.getByText('Attach a disk to be able to set a boot disk')).toBeVisible()
226228
})
229+
230+
// silly test but we've reintroduced this bug like 3 times
231+
test("polling doesn't close row actions menu", async ({ page }) => {
232+
await page.goto('/projects/mock-project/instances/db1')
233+
234+
// stop, but don't wait until the state has changed
235+
await page.getByRole('button', { name: 'Stop' }).click()
236+
await page.getByRole('button', { name: 'Confirm' }).click()
237+
await closeToast(page)
238+
239+
const menu = page.getByRole('menu')
240+
const stopped = page.getByText('statestopped')
241+
242+
await expect(menu).toBeHidden()
243+
await expect(stopped).toBeHidden()
244+
245+
await openRowActions(page, 'disk-1')
246+
await expect(stopped).toBeHidden() // still not stopped yet
247+
await expect(menu).toBeVisible()
248+
249+
// now we're stopped, which means polling has happened, but the
250+
// menu remains visible
251+
await expect(stopped).toBeVisible()
252+
await expect(menu).toBeVisible()
253+
254+
// now start it so we can check the non-boot disks table
255+
await page.getByRole('button', { name: 'Start' }).click()
256+
await page.getByRole('button', { name: 'Confirm' }).click()
257+
await closeToast(page)
258+
259+
const running = page.getByText('staterunning') // not running yet
260+
await expect(running).toBeHidden()
261+
await expect(menu).toBeHidden()
262+
263+
await openRowActions(page, 'disk-2')
264+
await expect(running).toBeHidden() // still not running yet
265+
await expect(menu).toBeVisible()
266+
267+
// state change means polling has happened. menu is still visible
268+
await expect(running).toBeVisible()
269+
await expect(menu).toBeVisible()
270+
})

test/e2e/instance.e2e.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,3 +240,7 @@ test('instance table', async ({ page }) => {
240240
state: expect.stringMatching(/^starting\d+s$/),
241241
})
242242
})
243+
244+
test("polling doesn't close row actions menu", async ({ page }) => {
245+
await page.goto('/projects/mock-project/instances/db1')
246+
})

0 commit comments

Comments
 (0)