Skip to content

Commit 645fca0

Browse files
committed
fix: address copilot review comments
1 parent cca39b4 commit 645fca0

3 files changed

Lines changed: 60 additions & 13 deletions

File tree

src/app/devices/device-toolbar/device-toolbar.component.html

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,19 +51,22 @@
5151
</button>
5252
<mat-divider style="height: 40px" vertical="true"></mat-divider>
5353
@if (!isLoading()()) {
54-
<mat-icon
55-
(click)="getPowerState()"
56-
style="padding: 0 18px 0 12px; cursor: pointer"
54+
<button
55+
mat-icon-button
56+
type="button"
57+
[matTooltip]="powerState() | translate"
58+
aria-label="Refresh power status"
59+
style="padding: 0 18px 0 12px"
5760
[style.color]="
5861
powerState() === 'deviceToolbar.power.on.value'
5962
? 'green'
6063
: powerState() === 'deviceToolbar.power.sleep.value'
6164
? 'yellow'
6265
: 'red'
6366
"
64-
[matTooltip]="powerState() | translate"
65-
>mode_standby</mat-icon
66-
>
67+
(click)="refreshPowerState()">
68+
<mat-icon>mode_standby</mat-icon>
69+
</button>
6770
}
6871
<mat-divider style="height: 40px" vertical="true"></mat-divider>
6972
<button mat-icon-button [matTooltip]="'devices.actions.powerUp.value' | translate" (click)="sendPowerAction(2)">

src/app/devices/device-toolbar/device-toolbar.component.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,16 @@ describe('DeviceToolbarComponent', () => {
138138
expect(component.isLoading()()).toBeFalse()
139139
})
140140

141+
it('should fetch power state using non-cached API on refresh', () => {
142+
devicesService.getPowerState.calls.reset()
143+
devicesService.getPowerStateCached.calls.reset()
144+
145+
component.refreshPowerState()
146+
147+
expect(devicesService.getPowerState).toHaveBeenCalledWith('guid')
148+
expect(devicesService.getPowerStateCached).not.toHaveBeenCalled()
149+
})
150+
141151
it('should navigate to device', async () => {
142152
fixture.componentRef.setInput('deviceId', '12345-pokli-456772')
143153
const routerSpy = spyOn(component.router, 'navigate')

src/app/devices/device-toolbar/device-toolbar.component.ts

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Component, OnInit, inject, signal, input, DestroyRef } from '@angular/c
77
import { catchError, finalize, switchMap } from 'rxjs/operators'
88
import { MatSnackBar } from '@angular/material/snack-bar'
99
import { Router } from '@angular/router'
10-
import { Observable, of, forkJoin } from 'rxjs'
10+
import { Observable, of, forkJoin, EMPTY } from 'rxjs'
1111
import { DevicesService } from '../devices.service'
1212
import SnackbarDefaults from '../../shared/config/snackBarDefault'
1313
import { AMTFeaturesResponse, BootDetails, Device, UserConsentResponse } from '../../../models/models'
@@ -164,7 +164,7 @@ export class DeviceToolbarComponent implements OnInit {
164164
this.device = data
165165
this.devicesService.device.next(this.device)
166166
this.isPinned.set(this.device?.certHash != null && this.device?.certHash !== '')
167-
this.getPowerState()
167+
this.loadPowerState()
168168
this.loadAMTFeatures()
169169
// react to AMT feature updates emitted by service
170170
this.devicesService
@@ -217,13 +217,48 @@ export class DeviceToolbarComponent implements OnInit {
217217
this.powerOptions.set(options)
218218
}
219219

220-
getPowerState(): void {
220+
private loadPowerState(): void {
221+
// Use cached to dedupe simultaneous requests (KVM deep-link + toolbar both loading).
221222
this.isLoading().set(true)
222-
// Bypass the cache so a manual refresh always fetches the latest state from
223-
// the device rather than returning a previously cached value.
223+
this.devicesService
224+
.getPowerStateCached(this.deviceId())
225+
.pipe(
226+
catchError(() => {
227+
// Network/backend failure; silently skip update and let UI show previous state.
228+
return EMPTY
229+
}),
230+
finalize(() => {
231+
this.isLoading().set(false)
232+
}),
233+
takeUntilDestroyed(this.destroyRef)
234+
)
235+
.subscribe((powerState) => {
236+
this.powerState.set(
237+
powerState.powerstate.toString() === '2'
238+
? 'deviceToolbar.power.on.value'
239+
: powerState.powerstate.toString() === '3' || powerState.powerstate.toString() === '4'
240+
? 'deviceToolbar.power.sleep.value'
241+
: 'deviceToolbar.power.off.value'
242+
)
243+
})
244+
}
245+
246+
refreshPowerState(): void {
247+
this.isLoading().set(true)
248+
// Bypass the cache so manual refresh always fetches the latest state from the
249+
// device rather than returning a previously cached value.
224250
this.devicesService
225251
.getPowerState(this.deviceId())
226-
.pipe(takeUntilDestroyed(this.destroyRef))
252+
.pipe(
253+
catchError(() => {
254+
// Network/backend failure; finalize will reset loading state.
255+
return EMPTY
256+
}),
257+
finalize(() => {
258+
this.isLoading().set(false)
259+
}),
260+
takeUntilDestroyed(this.destroyRef)
261+
)
227262
.subscribe((powerState) => {
228263
this.powerState.set(
229264
powerState.powerstate.toString() === '2'
@@ -232,7 +267,6 @@ export class DeviceToolbarComponent implements OnInit {
232267
? 'deviceToolbar.power.sleep.value'
233268
: 'deviceToolbar.power.off.value'
234269
)
235-
this.isLoading().set(false)
236270
})
237271
}
238272

0 commit comments

Comments
 (0)