Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to

### Fixed

- 🩺(project) reload app if front and back unsync #2276
- 🐛(frontend) fix patch and comments #2273
- 🐛(frontend) interlinking are exported correctly in print mode #2269
- 💬(frontend) add missing link in onboarding description #2233
Expand Down
1 change: 1 addition & 0 deletions src/backend/core/api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2841,6 +2841,7 @@ def get(self, request):
dict_settings[setting] = getattr(settings, setting)

dict_settings["theme_customization"] = self._load_theme_customization()
dict_settings["RELEASE_VERSION"] = settings.RELEASE

return drf.response.Response(dict_settings)

Expand Down
2 changes: 2 additions & 0 deletions src/backend/core/tests/test_api_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
FRONTEND_THEME="test-theme",
MEDIA_BASE_URL="http://testserver/",
POSTHOG_KEY={"id": "132456", "host": "https://eu.i.posthog-test.com"},
RELEASE="1.0.0",
SENTRY_DSN="https://sentry.test/123",
THEME_CUSTOMIZATION_FILE_PATH="",
)
Expand Down Expand Up @@ -77,6 +78,7 @@ def test_api_config(is_authenticated):
"LANGUAGE_CODE": "en-us",
"MEDIA_BASE_URL": "http://testserver/",
"POSTHOG_KEY": {"id": "132456", "host": "https://eu.i.posthog-test.com"},
"RELEASE_VERSION": "1.0.0",
"SENTRY_DSN": "https://sentry.test/123",
"TRASHBIN_CUTOFF_DAYS": 30,
"theme_customization": {},
Expand Down
30 changes: 29 additions & 1 deletion src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { expect, test } from '@playwright/test';

import { createDoc, getCurrentConfig, verifyDocName } from './utils-common';
import {
createDoc,
getCurrentConfig,
overrideConfig,
verifyDocName,
} from './utils-common';
import { writeInEditor } from './utils-editor';
import { SignIn, expectLoginPage } from './utils-signin';
import { createRootSubPage } from './utils-sub-pages';
Expand Down Expand Up @@ -145,6 +150,29 @@ test.describe('Doc Routing', () => {
);
await expect(page).toHaveTitle(/401 Unauthorized - Docs/);
});

test('checks redirect if unsync version', async ({ page }) => {
await overrideConfig(page, {
RELEASE_VERSION: '0.0.0',
});

let counterReload = 0;
await page.route(/.*\/users\/me\/$/, async (route) => {
counterReload += 1;
await route.continue();
});

await page.waitForTimeout(1000);

// The sessionStorage guard should be set to the mismatched backend version.
const reloadVersion = await page.evaluate(() =>
sessionStorage.getItem('reload-version'),
);
expect(reloadVersion).toBe('0.0.0');

// The page should have reloaded once
expect(counterReload).toBe(2);
});
Comment on lines +159 to +175

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the reload assertion deterministic to avoid flaky CI failures.

Line 165 uses a fixed sleep, and Line 174 expects an exact /users/me/ count. This is timing-sensitive and can intermittently fail with retries/background refetches. Also, register the route before triggering mismatch logic so the first hit is never missed.

Suggested stabilization
 test('checks redirect if unsync version', async ({ page }) => {
+  let counterReload = 0;
+  await page.route(/.*\/users\/me\/$/, async (route) => {
+    counterReload += 1;
+    await route.continue();
+  });
+
   await overrideConfig(page, {
     RELEASE_VERSION: '0.0.0',
   });
 
-  let counterReload = 0;
-  await page.route(/.*\/users\/me\/$/, async (route) => {
-    counterReload += 1;
-    await route.continue();
-  });
-
-  await page.waitForTimeout(1000);
-
-  // The sessionStorage guard should be set to the mismatched backend version.
-  const reloadVersion = await page.evaluate(() =>
-    sessionStorage.getItem('reload-version'),
-  );
-  expect(reloadVersion).toBe('0.0.0');
-
-  // The page should have reloaded once
-  expect(counterReload).toBe(2);
+  await expect
+    .poll(
+      () => page.evaluate(() => sessionStorage.getItem('reload-version')),
+      { timeout: 10000 },
+    )
+    .toBe('0.0.0');
+
+  // At least 2 calls proves initial load + post-reload load.
+  await expect.poll(() => counterReload, { timeout: 10000 }).toBeGreaterThanOrEqual(2);
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts` around lines
159 - 175, Register the page.route handler for /users/me/ before performing the
actions that trigger a version-mismatch reload, replace the fixed waitForTimeout
with a deterministic wait that polls sessionStorage (e.g. await
page.waitForFunction(() => sessionStorage.getItem('reload-version') ===
'0.0.0')), and then assert the route hit count in a stable way (either assert
counterReload >= 2 or assert counterReload === 2 only if you guarantee the first
hit cannot be missed). Update references: the page.route handler that increments
counterReload, the sessionStorage.getItem('reload-version') check, and the final
expect on counterReload.

});

test.describe('Doc Routing: Not logged', () => {
Expand Down
2 changes: 2 additions & 0 deletions src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import path from 'path';
import { Locator, Page, TestInfo, expect } from '@playwright/test';

import theme_customization from '../../../../../backend/impress/configuration/theme/default.json';
import { version as packageJsonVersion } from '../../package.json';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the Impress package version, not the E2E package version, for RELEASE_VERSION.

../../package.json resolves to the e2e app package, which can diverge from NEXT_PUBLIC_APP_VERSION (built from apps/impress/package.json) and cause false mismatch/reload behavior in tests.

✅ Suggested fix
-import { version as packageJsonVersion } from '../../package.json';
+import { version as packageJsonVersion } from '../../../impress/package.json';

Also applies to: 44-44

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts` at line 7, The
RELEASE_VERSION is currently imported from the e2e app package.json
(packageJsonVersion from '../../package.json') which can diverge from
NEXT_PUBLIC_APP_VERSION; change the import to read the Impress app's
package.json (the apps/impress package.json used to build
NEXT_PUBLIC_APP_VERSION) and use that value for RELEASE_VERSION instead, and
update the second occurrence at the other import/site (line referenced by the
review) so both places reference the Impress package.json version; locate
occurrences by the identifier RELEASE_VERSION and the import named
packageJsonVersion and replace their source to the Impress package.json.


export type BrowserName = 'chromium' | 'firefox' | 'webkit';
export const BROWSERS: BrowserName[] = ['chromium', 'webkit', 'firefox'];
Expand Down Expand Up @@ -40,6 +41,7 @@ export const CONFIG = {
],
LANGUAGE_CODE: 'en-us',
POSTHOG_KEY: {},
RELEASE_VERSION: packageJsonVersion,
SENTRY_DSN: null,
TRASHBIN_CUTOFF_DAYS: 30,
theme_customization,
Expand Down
3 changes: 3 additions & 0 deletions src/frontend/apps/impress/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ const crypto = require('crypto');

const { InjectManifest } = require('workbox-webpack-plugin');

const { version } = require('./package.json');

const buildId = crypto.randomBytes(256).toString('hex').slice(0, 8);

/** @type {import('next').NextConfig} */
Expand All @@ -25,6 +27,7 @@ const nextConfig = {
generateBuildId: () => buildId,
env: {
NEXT_PUBLIC_BUILD_ID: buildId,
NEXT_PUBLIC_APP_VERSION: version,
},
/**
* In dev mode, Next.js doesn't use Webpack, but Turbopack.
Expand Down
26 changes: 26 additions & 0 deletions src/frontend/apps/impress/src/core/config/ConfigProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,32 @@ export const ConfigProvider = ({ children }: PropsWithChildren) => {
});
}, [conf?.CRISP_WEBSITE_ID]);

useEffect(() => {
const frontendVersion = process.env.NEXT_PUBLIC_APP_VERSION;

if (
!conf?.RELEASE_VERSION ||
!frontendVersion ||
conf.RELEASE_VERSION === frontendVersion
) {
return;
}

// Avoid infinite reload loops: only reload once per backend version
const RELOAD_VERSION_KEY = 'reload-version';
try {
const reloadedForVersion = sessionStorage.getItem(RELOAD_VERSION_KEY);
if (reloadedForVersion === conf.RELEASE_VERSION) {
return;
}

sessionStorage.setItem(RELOAD_VERSION_KEY, conf.RELEASE_VERSION);
window.location.reload();
} catch {
console.warn('Failed to access sessionStorage for version reload logic');
}
}, [conf?.RELEASE_VERSION]);

if (!conf) {
return (
<Box $height="100vh" $width="100vw" $align="center" $justify="center">
Expand Down
7 changes: 4 additions & 3 deletions src/frontend/apps/impress/src/core/config/api/useConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export interface ConfigResponse {
LANGUAGE_CODE: string;
MEDIA_BASE_URL?: string;
POSTHOG_KEY?: PostHogConf;
RELEASE_VERSION: string;
SENTRY_DSN?: string;
TRASHBIN_CUTOFF_DAYS?: number;
theme_customization?: ThemeCustomization;
Expand Down Expand Up @@ -94,13 +95,13 @@ export const KEY_CONFIG = 'config';

export function useConfig() {
const cachedData = getCachedTranslation();
const oneHour = 1000 * 60 * 60;
const staleTime = 1000 * 60 * 5;

return useQuery<ConfigResponse, APIError, ConfigResponse>({
queryKey: [KEY_CONFIG],
queryFn: () => getConfig(),
initialData: cachedData,
staleTime: oneHour,
initialDataUpdatedAt: Date.now() - oneHour, // Force initial data to be considered stale
staleTime,
initialDataUpdatedAt: Date.now() - staleTime, // Force initial data to be considered stale
});
}
6 changes: 6 additions & 0 deletions src/frontend/apps/impress/src/services/PosthogAnalytic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ export function PostHogProvider({
if (process.env.NODE_ENV === 'development') {
posthogInstance.debug();
}

if (process.env.NEXT_PUBLIC_APP_VERSION) {
posthogInstance.register({
app_version: process.env.NEXT_PUBLIC_APP_VERSION,
});
}
},
capture_pageview: false,
capture_pageleave: true,
Expand Down
Loading