From f7f7b8790e0f43935e315f4be2b88e251ffe225c Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:00:43 +0100 Subject: [PATCH] fix update notification visibility and install flicker (#6776) # Description of Changes - Closes #6754 - Update popup now hidden on mobile, for non-admins, and never on SaaS - Respects admin "Show Update Notifications" setting (`showUpdate` / `showUpdateOnlyAdmin`, now default on) - Fixes update modal flickering during desktop install --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details. --- .../common/model/ApplicationProperties.java | 4 +- .../controller/api/misc/ConfigController.java | 9 ++ .../src/main/resources/settings.yml.template | 4 +- .../api/misc/ConfigControllerMoreTest.java | 3 + .../api/misc/ConfigControllerTest.java | 3 + .../service/AppUpdateAuthServiceTest.java | 93 +++++++++++++++++++ .../components/shared/UpdateModal.test.tsx | 66 +++++++++++++ .../core/components/shared/UpdateModal.tsx | 31 ++++--- .../components/shared/UpdateStartupPopup.tsx | 9 +- .../components/shared/updatePopupGate.test.ts | 33 +++++++ .../core/components/shared/updatePopupGate.ts | 10 ++ frontend/editor/src/core/types/appConfig.ts | 1 + .../components/shared/UpdateStartupPopup.tsx | 5 + 13 files changed, 255 insertions(+), 16 deletions(-) create mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/security/service/AppUpdateAuthServiceTest.java create mode 100644 frontend/editor/src/core/components/shared/UpdateModal.test.tsx create mode 100644 frontend/editor/src/core/components/shared/updatePopupGate.test.ts create mode 100644 frontend/editor/src/core/components/shared/updatePopupGate.ts create mode 100644 frontend/editor/src/saas/components/shared/UpdateStartupPopup.tsx diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index b955a2fe32..e80d6b2360 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -869,8 +869,8 @@ public class ApplicationProperties { public static class System { private String defaultLocale; private boolean googlevisibility; - private boolean showUpdate; - private boolean showUpdateOnlyAdmin; + private boolean showUpdate = true; + private boolean showUpdateOnlyAdmin = true; private boolean showSettingsWhenNoLogin = true; private boolean customHTMLFiles; private String tessdataDir; diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ConfigController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ConfigController.java index 1b2fcaddfa..3ea525a64e 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ConfigController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ConfigController.java @@ -22,6 +22,7 @@ import stirling.software.SPDF.config.InitialSetup; import stirling.software.SPDF.controller.api.security.TimestampController; import stirling.software.common.annotations.api.ConfigApi; import stirling.software.common.configuration.AppConfig; +import stirling.software.common.configuration.interfaces.ShowAdminInterface; import stirling.software.common.model.ApplicationProperties; import stirling.software.common.service.ServerCertificateServiceInterface; import stirling.software.common.service.UserServiceInterface; @@ -37,6 +38,7 @@ public class ConfigController { private final EndpointConfiguration endpointConfiguration; private final ServerCertificateServiceInterface serverCertificateService; private final UserServiceInterface userService; + private final ShowAdminInterface showAdmin; private final stirling.software.common.service.LicenseServiceInterface licenseService; private final stirling.software.SPDF.config.ExternalAppDepConfig externalAppDepConfig; @@ -48,6 +50,8 @@ public class ConfigController { ServerCertificateServiceInterface serverCertificateService, @org.springframework.beans.factory.annotation.Autowired(required = false) UserServiceInterface userService, + @org.springframework.beans.factory.annotation.Autowired(required = false) + ShowAdminInterface showAdmin, @org.springframework.beans.factory.annotation.Autowired(required = false) stirling.software.common.service.LicenseServiceInterface licenseService, stirling.software.SPDF.config.ExternalAppDepConfig externalAppDepConfig) { @@ -56,6 +60,7 @@ public class ConfigController { this.endpointConfiguration = endpointConfiguration; this.serverCertificateService = serverCertificateService; this.userService = userService; + this.showAdmin = showAdmin; this.licenseService = licenseService; this.externalAppDepConfig = externalAppDepConfig; } @@ -315,6 +320,10 @@ public class ConfigController { configData.put( "enableAlphaFunctionality", applicationProperties.getSystem().isEnableAlphaFunctionality()); + boolean shouldShowUpdate = + applicationProperties.getSystem().isShowUpdate() + && (showAdmin == null || showAdmin.getShowUpdateOnlyAdmins()); + configData.put("shouldShowUpdate", shouldShowUpdate); configData.put( "enableAnalytics", applicationProperties.getSystem().getEnableAnalytics()); configData.put("enablePosthog", applicationProperties.getSystem().getEnablePosthog()); diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index 9bce7d1c55..1e9e103742 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -170,8 +170,8 @@ system: defaultLocale: "" # force a default language for new users (e.g. 'en-US', 'de-DE'). Empty string auto-detects from the browser, falling back to en-US googlevisibility: false # 'true' to allow Google visibility (via robots.txt), 'false' to disallow enableAlphaFunctionality: false # set to enable functionality which might need more testing before it fully goes live (this feature might make no changes) - showUpdate: false # see when a new update is available - showUpdateOnlyAdmin: false # only admins can see when a new update is available, depending on showUpdate it must be set to 'true' + showUpdate: true # see when a new update is available + showUpdateOnlyAdmin: true # only admins can see when a new update is available, depending on showUpdate it must be set to 'true' showSettingsWhenNoLogin: true # set to 'false' to hide settings button when login is disabled (enableLogin: false). Only applies when login is disabled. customHTMLFiles: false # enable to have files placed in /customFiles/templates override the existing template HTML files tessdataDir: "" # path to the directory containing the Tessdata files. This setting is relevant for Windows systems. For Windows users, this path should be adjusted to point to the appropriate directory where the Tessdata files are stored. diff --git a/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerMoreTest.java b/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerMoreTest.java index e62bf4fc85..d0974f07c3 100644 --- a/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerMoreTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerMoreTest.java @@ -26,6 +26,7 @@ import jakarta.servlet.http.HttpServletRequest; import stirling.software.SPDF.config.EndpointConfiguration; import stirling.software.SPDF.config.ExternalAppDepConfig; import stirling.software.common.configuration.AppConfig; +import stirling.software.common.configuration.interfaces.ShowAdminInterface; import stirling.software.common.model.ApplicationProperties; import stirling.software.common.service.LicenseServiceInterface; import stirling.software.common.service.ServerCertificateServiceInterface; @@ -45,6 +46,7 @@ class ConfigControllerMoreTest { @Mock private EndpointConfiguration endpointConfiguration; @Mock private ServerCertificateServiceInterface serverCertificateService; @Mock private UserServiceInterface userService; + @Mock private ShowAdminInterface showAdmin; @Mock private LicenseServiceInterface licenseService; @Mock private ExternalAppDepConfig externalAppDepConfig; @Mock private AppConfig appConfig; @@ -70,6 +72,7 @@ class ConfigControllerMoreTest { endpointConfiguration, serverCertificateService, userService, + showAdmin, licenseService, externalAppDepConfig); } diff --git a/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerTest.java b/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerTest.java index cab1622c69..faf5c70985 100644 --- a/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/controller/api/misc/ConfigControllerTest.java @@ -21,6 +21,7 @@ import stirling.software.SPDF.config.EndpointConfiguration; import stirling.software.SPDF.config.EndpointConfiguration.DisableReason; import stirling.software.SPDF.config.EndpointConfiguration.EndpointAvailability; import stirling.software.common.configuration.AppConfig; +import stirling.software.common.configuration.interfaces.ShowAdminInterface; import stirling.software.common.model.ApplicationProperties; import stirling.software.common.model.ApplicationProperties.System; import stirling.software.common.service.LicenseServiceInterface; @@ -35,6 +36,7 @@ class ConfigControllerTest { @Mock private EndpointConfiguration endpointConfiguration; @Mock private ServerCertificateServiceInterface serverCertificateService; @Mock private UserServiceInterface userService; + @Mock private ShowAdminInterface showAdmin; @Mock private LicenseServiceInterface licenseService; private ConfigController configController; @@ -48,6 +50,7 @@ class ConfigControllerTest { endpointConfiguration, serverCertificateService, userService, + showAdmin, licenseService, mock(stirling.software.SPDF.config.ExternalAppDepConfig.class)); } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/AppUpdateAuthServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/AppUpdateAuthServiceTest.java new file mode 100644 index 0000000000..ccd6582409 --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/AppUpdateAuthServiceTest.java @@ -0,0 +1,93 @@ +package stirling.software.proprietary.security.service; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import java.util.Optional; + +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; + +import stirling.software.common.model.ApplicationProperties; +import stirling.software.proprietary.security.database.repository.UserRepository; +import stirling.software.proprietary.security.model.User; + +class AppUpdateAuthServiceTest { + + private final UserRepository userRepository = mock(UserRepository.class); + + private AppUpdateAuthService service(boolean showUpdate, boolean onlyAdmin) { + ApplicationProperties props = new ApplicationProperties(); + ApplicationProperties.System system = new ApplicationProperties.System(); + system.setShowUpdate(showUpdate); + system.setShowUpdateOnlyAdmin(onlyAdmin); + props.setSystem(system); + return new AppUpdateAuthService(userRepository, props); + } + + private void withAuthentication(Authentication auth, Runnable assertions) { + try (MockedStatic holder = mockStatic(SecurityContextHolder.class)) { + SecurityContext context = mock(SecurityContext.class); + when(context.getAuthentication()).thenReturn(auth); + holder.when(SecurityContextHolder::getContext).thenReturn(context); + assertions.run(); + } + } + + private Authentication authenticatedAs(String username, String roles) { + Authentication auth = mock(Authentication.class); + when(auth.isAuthenticated()).thenReturn(true); + when(auth.getName()).thenReturn(username); + User user = mock(User.class); + when(user.getRolesAsString()).thenReturn(roles); + when(userRepository.findByUsername(username)).thenReturn(Optional.of(user)); + return auth; + } + + @Test + void hidesForEveryoneWhenShowUpdateOff() { + withAuthentication( + authenticatedAs("admin", "ROLE_ADMIN"), + () -> assertFalse(service(false, true).getShowUpdateOnlyAdmins())); + } + + @Test + void showsToAllAuthenticatedWhenNotAdminOnly() { + withAuthentication( + authenticatedAs("bob", "ROLE_USER"), + () -> assertTrue(service(true, false).getShowUpdateOnlyAdmins())); + } + + @Test + void hidesFromAnonymousWhenAdminOnly() { + withAuthentication(null, () -> assertFalse(service(true, true).getShowUpdateOnlyAdmins())); + } + + @Test + void hidesFromAnonymousUserPrincipalWhenAdminOnly() { + Authentication auth = mock(Authentication.class); + when(auth.isAuthenticated()).thenReturn(true); + when(auth.getName()).thenReturn("anonymousUser"); + withAuthentication(auth, () -> assertFalse(service(true, true).getShowUpdateOnlyAdmins())); + } + + @Test + void showsToAdminWhenAdminOnly() { + withAuthentication( + authenticatedAs("admin", "ROLE_ADMIN"), + () -> assertTrue(service(true, true).getShowUpdateOnlyAdmins())); + } + + @Test + void hidesFromNonAdminWhenAdminOnly() { + withAuthentication( + authenticatedAs("bob", "ROLE_USER"), + () -> assertFalse(service(true, true).getShowUpdateOnlyAdmins())); + } +} diff --git a/frontend/editor/src/core/components/shared/UpdateModal.test.tsx b/frontend/editor/src/core/components/shared/UpdateModal.test.tsx new file mode 100644 index 0000000000..e2026b33bb --- /dev/null +++ b/frontend/editor/src/core/components/shared/UpdateModal.test.tsx @@ -0,0 +1,66 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { MantineProvider } from "@mantine/core"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { MachineInfo, UpdateSummary } from "@app/services/updateService"; + +const getFullUpdateInfo = vi.fn(); + +vi.mock("@app/services/updateService", () => ({ + updateService: { + getFullUpdateInfo: (...args: unknown[]) => getFullUpdateInfo(...args), + getDownloadUrl: () => null, + compareVersions: (a: string, b: string) => (a === b ? 0 : a > b ? 1 : -1), + }, +})); + +import UpdateModal from "@app/components/shared/UpdateModal"; + +const summary: UpdateSummary = { + latest_version: "2.0.0", + max_priority: "normal", + any_breaking: false, +}; + +const machine = (): MachineInfo => ({ + machineType: "Client-win", + activeSecurity: false, + licenseType: "NORMAL", +}); + +function wrap(machineInfo: MachineInfo) { + return ( + + {}} + currentVersion="1.0.0" + updateSummary={summary} + machineInfo={machineInfo} + /> + + ); +} + +describe("UpdateModal", () => { + afterEach(() => getFullUpdateInfo.mockReset()); + + it("does not refetch version info when machineInfo identity changes but values are equal", async () => { + getFullUpdateInfo.mockResolvedValue({ + latest_version: "2.0.0", + new_versions: [], + }); + + const { rerender } = render(wrap(machine())); + + await waitFor(() => + expect( + screen.queryByText("update.loadingDetailedInfo"), + ).not.toBeInTheDocument(), + ); + expect(getFullUpdateInfo).toHaveBeenCalledTimes(1); + + rerender(wrap(machine())); + + await waitFor(() => expect(getFullUpdateInfo).toHaveBeenCalledTimes(1)); + }); +}); diff --git a/frontend/editor/src/core/components/shared/UpdateModal.tsx b/frontend/editor/src/core/components/shared/UpdateModal.tsx index 1399325b03..c18549360c 100644 --- a/frontend/editor/src/core/components/shared/UpdateModal.tsx +++ b/frontend/editor/src/core/components/shared/UpdateModal.tsx @@ -130,18 +130,27 @@ const UpdateModal: React.FC = ({ new Set([0]), ); + const { machineType, activeSecurity, licenseType } = machineInfo; useEffect(() => { - if (opened) { - setLoading(true); - setExpandedVersions(new Set([0])); - updateService - .getFullUpdateInfo(currentVersion, machineInfo) - .then((info) => { - setFullUpdateInfo(info); - setLoading(false); - }); - } - }, [opened, currentVersion, machineInfo]); + if (!opened) return; + let cancelled = false; + setLoading(true); + setExpandedVersions(new Set([0])); + updateService + .getFullUpdateInfo(currentVersion, { + machineType, + activeSecurity, + licenseType, + }) + .then((info) => { + if (cancelled) return; + setFullUpdateInfo(info); + setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [opened, currentVersion, machineType, activeSecurity, licenseType]); const toggleVersion = (index: number) => { setExpandedVersions((prev) => { diff --git a/frontend/editor/src/core/components/shared/UpdateStartupPopup.tsx b/frontend/editor/src/core/components/shared/UpdateStartupPopup.tsx index 62f6dbb999..fd3de26bdc 100644 --- a/frontend/editor/src/core/components/shared/UpdateStartupPopup.tsx +++ b/frontend/editor/src/core/components/shared/UpdateStartupPopup.tsx @@ -1,7 +1,9 @@ import { useEffect, useRef, useState } from "react"; import { useAppConfig } from "@app/contexts/AppConfigContext"; import { useFrontendVersionInfo } from "@app/hooks/useFrontendVersionInfo"; +import { useIsMobile } from "@app/hooks/useIsMobile"; import { updateService, type UpdateSummary } from "@app/services/updateService"; +import { isUpdatePopupAllowed } from "@app/components/shared/updatePopupGate"; import UpdateModal from "@app/components/shared/UpdateModal"; /** @@ -26,6 +28,7 @@ const SNOOZE_DURATION_MS = 24 * 60 * 60 * 1000; */ export function UpdateStartupPopup() { const { config } = useAppConfig(); + const isMobile = useIsMobile(); const { appVersion } = useFrontendVersionInfo(config?.appVersion); // The version to compare against the latest. Prefer the frontend version @@ -39,10 +42,12 @@ export function UpdateStartupPopup() { const [showModal, setShowModal] = useState(false); const hasChecked = useRef(false); + const allowed = isUpdatePopupAllowed(config, isMobile); + useEffect(() => { if (hasChecked.current) return; if (!currentVersion) return; - // Don't even schedule the timer until we have a version to compare. + if (!allowed) return; hasChecked.current = true; const timer = setTimeout(async () => { @@ -79,12 +84,14 @@ export function UpdateStartupPopup() { return () => clearTimeout(timer); }, [ + allowed, currentVersion, config?.machineType, config?.activeSecurity, config?.license, ]); + if (!allowed) return null; if (!updateSummary || !currentVersion) return null; const machineInfo = { diff --git a/frontend/editor/src/core/components/shared/updatePopupGate.test.ts b/frontend/editor/src/core/components/shared/updatePopupGate.test.ts new file mode 100644 index 0000000000..b42bdf55b5 --- /dev/null +++ b/frontend/editor/src/core/components/shared/updatePopupGate.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import type { AppConfig } from "@app/types/appConfig"; +import { isUpdatePopupAllowed } from "@app/components/shared/updatePopupGate"; + +describe("isUpdatePopupAllowed", () => { + it("returns false until config has loaded", () => { + expect(isUpdatePopupAllowed(null, false)).toBe(false); + }); + + it("shows on desktop when the backend allows it", () => { + expect( + isUpdatePopupAllowed({ shouldShowUpdate: true } as AppConfig, false), + ).toBe(true); + }); + + it("never shows on a mobile / narrow viewport", () => { + expect( + isUpdatePopupAllowed({ shouldShowUpdate: true } as AppConfig, true), + ).toBe(false); + }); + + it("hides when the backend says not to (non-admin / showUpdate disabled)", () => { + expect( + isUpdatePopupAllowed({ shouldShowUpdate: false } as AppConfig, false), + ).toBe(false); + }); + + it("fails closed when the backend omits the decision (e.g. 401 fallback)", () => { + expect( + isUpdatePopupAllowed({ enableLogin: true } as AppConfig, false), + ).toBe(false); + }); +}); diff --git a/frontend/editor/src/core/components/shared/updatePopupGate.ts b/frontend/editor/src/core/components/shared/updatePopupGate.ts new file mode 100644 index 0000000000..c086e7b664 --- /dev/null +++ b/frontend/editor/src/core/components/shared/updatePopupGate.ts @@ -0,0 +1,10 @@ +import type { AppConfig } from "@app/types/appConfig"; + +export function isUpdatePopupAllowed( + config: AppConfig | null, + isMobile: boolean, +): boolean { + if (!config) return false; + if (isMobile) return false; + return config.shouldShowUpdate === true; +} diff --git a/frontend/editor/src/core/types/appConfig.ts b/frontend/editor/src/core/types/appConfig.ts index 55d73d7e0f..c73faebd8c 100644 --- a/frontend/editor/src/core/types/appConfig.ts +++ b/frontend/editor/src/core/types/appConfig.ts @@ -13,6 +13,7 @@ export interface AppConfig { enableOAuth?: boolean; enableSaml?: boolean; isAdmin?: boolean; + shouldShowUpdate?: boolean; enableAlphaFunctionality?: boolean; enableAnalytics?: boolean | null; enablePosthog?: boolean | null; diff --git a/frontend/editor/src/saas/components/shared/UpdateStartupPopup.tsx b/frontend/editor/src/saas/components/shared/UpdateStartupPopup.tsx new file mode 100644 index 0000000000..1a53359afe --- /dev/null +++ b/frontend/editor/src/saas/components/shared/UpdateStartupPopup.tsx @@ -0,0 +1,5 @@ +export function UpdateStartupPopup() { + return null; +} + +export default UpdateStartupPopup;