Skip to content

Commit 12beb1e

Browse files
authored
fix: network rollback status persists after acknowledgment (#87)
## Summary Fixes the issue where the network rollback notification incorrectly reappears on subsequent logins after being acknowledged. ## Changes - Extended `unauth_post!` macro with Pattern 0 for simple POST without body - Changed `handle_ack_rollback` to use `unauth_post!` instead of `auth_post!` - Improved error handling in `clear_rollback_occurred` with proper logging - Added E2E test to verify fix ## Root Cause The rollback dialog appears before login (in `App.vue` `onMounted`), so no auth token was available when clicking OK. This caused the `/ack-rollback` request to silently fail, leaving the backend marker file in place. --------- Signed-off-by: Jan Zachmann <50990105+JanZachmann@users.noreply.github.com>
1 parent 1859f89 commit 12beb1e

File tree

4 files changed

+123
-3
lines changed

4 files changed

+123
-3
lines changed

src/app/src/macros.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,21 @@ pub use crate::http_helpers::{
4242
/// Macro for unauthenticated POST requests with standard error handling.
4343
/// Requires domain parameters for event wrapping.
4444
///
45+
/// # Patterns
46+
///
47+
/// Pattern 0: Simple POST without body (status only)
48+
/// ```ignore
49+
/// unauth_post!(Device, DeviceEvent, model, "/ack-rollback", AckRollbackResponse, "Acknowledge rollback")
50+
/// ```
51+
///
52+
/// Pattern 1: POST with JSON body expecting JSON response
53+
/// ```ignore
54+
/// unauth_post!(Auth, AuthEvent, model, "/endpoint", Response, "Action",
55+
/// body_json: &request,
56+
/// expect_json: ResponseType
57+
/// )
58+
/// ```
59+
///
4560
/// Pattern 2: POST with JSON body expecting status only
4661
/// ```ignore
4762
/// unauth_post!(Auth, AuthEvent, model, "/set-password", SetPasswordResponse, "Set password",
@@ -58,6 +73,23 @@ pub use crate::http_helpers::{
5873
/// ```
5974
#[macro_export]
6075
macro_rules! unauth_post {
76+
// Pattern 0: Simple POST without body (status only)
77+
($domain:ident, $domain_event:ident, $model:expr, $endpoint:expr, $response_event:ident, $action:expr) => {{
78+
$model.start_loading();
79+
let cmd = crux_core::Command::all([
80+
crux_core::render::render(),
81+
$crate::HttpCmd::post($crate::build_url($endpoint))
82+
.build()
83+
.then_send(|result| {
84+
let event_result = $crate::process_status_response($action, result);
85+
$crate::events::Event::$domain(
86+
$crate::events::$domain_event::$response_event(event_result),
87+
)
88+
}),
89+
]);
90+
cmd
91+
}};
92+
6193
// Pattern 1: POST with JSON body expecting JSON response
6294
($domain:ident, $domain_event:ident, $model:expr, $endpoint:expr, $response_event:ident, $action:expr, body_json: $body:expr, expect_json: $response_type:ty) => {{
6395
$model.start_loading();

src/app/src/update/device/network.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::auth_post;
44
use crate::events::{DeviceEvent, Event, UiEvent};
55
use crate::http_get_silent;
66
use crate::model::Model;
7+
use crate::unauth_post;
78
use crate::types::{
89
HealthcheckInfo, NetworkChangeState, NetworkConfigRequest, NetworkFormData, NetworkFormState,
910
OverlaySpinnerState,
@@ -299,7 +300,9 @@ pub fn handle_ack_rollback(model: &mut Model) -> Command<Effect, Event> {
299300
}
300301

301302
// Send POST request to backend to clear the marker file
302-
auth_post!(
303+
// Note: Using unauth_post instead of auth_post because this may be called before login
304+
// (the rollback notification appears in App.vue onMounted, before authentication)
305+
unauth_post!(
303306
Device,
304307
DeviceEvent,
305308
model,

src/backend/src/services/network.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,15 @@ impl NetworkConfigService {
253253

254254
/// Clear the rollback occurred marker (called when UI acknowledges it)
255255
pub fn clear_rollback_occurred() {
256-
let _ = fs::remove_file(network_rollback_occurred_file!());
257-
info!("rollback occurred marker cleared");
256+
let marker_file = network_rollback_occurred_file!();
257+
info!("Attempting to clear rollback occurred marker at: {marker_file:?}");
258+
match fs::remove_file(marker_file) {
259+
Ok(()) => info!("Successfully removed rollback occurred marker file"),
260+
Err(e) if e.kind() == ErrorKind::NotFound => {
261+
info!("Rollback occurred marker file does not exist (already cleared)")
262+
}
263+
Err(e) => error!("Failed to remove rollback occurred marker file: {e}"),
264+
}
258265
}
259266

260267
/// Mark that a rollback has occurred (sets marker file)
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { test, expect } from '@playwright/test';
2+
import { mockConfig, mockLoginSuccess, mockRequireSetPassword } from './fixtures/mock-api';
3+
import { publishToCentrifugo } from './fixtures/centrifugo';
4+
5+
test.describe('Network Rollback Status', () => {
6+
test('rollback status is cleared after ack and does not reappear on re-login', async ({ page, context }) => {
7+
// Track healthcheck calls
8+
let healthcheckRollbackStatus = true;
9+
10+
await mockConfig(page);
11+
await mockLoginSuccess(page);
12+
await mockRequireSetPassword(page);
13+
14+
// Mock healthcheck with rollback occurred status
15+
await page.route('**/healthcheck', async (route) => {
16+
await route.fulfill({
17+
status: 200,
18+
contentType: 'application/json',
19+
body: JSON.stringify({
20+
version_info: {
21+
required: '>=0.39.0',
22+
current: '0.40.0',
23+
mismatch: false,
24+
},
25+
update_validation_status: {
26+
status: 'valid',
27+
},
28+
network_rollback_occurred: healthcheckRollbackStatus,
29+
}),
30+
});
31+
});
32+
33+
// Mock ack-rollback endpoint
34+
await page.route('**/ack-rollback', async (route) => {
35+
if (route.request().method() === 'POST') {
36+
// Simulate clearing the rollback status on the backend
37+
healthcheckRollbackStatus = false;
38+
await route.fulfill({
39+
status: 200,
40+
});
41+
}
42+
});
43+
44+
// Step 1: Navigate to page - rollback notification appears on mount (before login)
45+
await page.goto('/');
46+
47+
// The rollback notification dialog appears immediately (from healthcheck in onMounted)
48+
await expect(page.getByText('Network Settings Rolled Back')).toBeVisible({ timeout: 10000 });
49+
50+
// Step 2: Acknowledge the rollback message
51+
// This should call /ack-rollback (now without auth requirement) and clear the backend marker
52+
await page.getByRole('button', { name: /ok/i }).click();
53+
await expect(page.getByText('Network Settings Rolled Back')).not.toBeVisible();
54+
55+
// Wait a moment for the async POST to /ack-rollback to complete
56+
await page.waitForTimeout(500);
57+
58+
// Now we can log in
59+
await page.getByPlaceholder(/enter your password/i).fill('password');
60+
await page.getByRole('button', { name: /log in/i }).click();
61+
await expect(page.getByText('Common Info')).toBeVisible({ timeout: 10000 });
62+
63+
// Step 3: Reload the page to simulate logout and re-login
64+
await page.reload();
65+
66+
// The rollback notification should NOT appear again because we acknowledged it
67+
// and the /ack-rollback call cleared the backend marker file
68+
await expect(page.getByText('Network Settings Rolled Back')).not.toBeVisible({ timeout: 3000 });
69+
70+
// Can proceed with login
71+
await page.getByPlaceholder(/enter your password/i).fill('password');
72+
await page.getByRole('button', { name: /log in/i }).click();
73+
await expect(page.getByText('Common Info')).toBeVisible({ timeout: 10000 });
74+
75+
// Verify no rollback notification
76+
await expect(page.getByText('Network Settings Rolled Back')).not.toBeVisible();
77+
});
78+
});

0 commit comments

Comments
 (0)