From 590c50c1c6625aa43dd89c265945ee3e613f12ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Wed, 5 Aug 2026 15:39:59 +0200 Subject: [PATCH 01/19] add translation keys --- web/messages/en/modal.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/web/messages/en/modal.json b/web/messages/en/modal.json index 0789db0a5..d72348cb3 100644 --- a/web/messages/en/modal.json +++ b/web/messages/en/modal.json @@ -221,9 +221,15 @@ "modal_assign_posture_check_locations_error": "Failed to assign locations to the posture check", "modal_delete_posture_check_title": "Delete posture check", "modal_delete_posture_check_content_empty": "Are you sure you want to delete this check?", - "modal_delete_posture_check_content": "Are you sure you want to delete this check? It’s currently used in {locations}. Removing it may change access criteria for users in these locations.", + "modal_delete_posture_check_content": "Are you sure you want to delete this check? It’s currently used in {locations}. Removing it may change access criteria for users in these locations and disconnect active VPN client sessions there.", "modal_delete_posture_check_success": "Posture check deleted", "modal_delete_posture_check_error": "Failed to delete posture check", + "modal_posture_assignment_warning_title": "Confirm posture check changes", + "modal_posture_assignment_warning_body_location": "These changes may disconnect active VPN client sessions for this location.\n\n{changes}", + "modal_posture_assignment_warning_body_postures": "These changes may disconnect active VPN client sessions for the affected locations.\n\n{changes}", + "modal_posture_assignment_warning_added": "Added:", + "modal_posture_assignment_warning_removed": "Removed:", + "modal_posture_rules_warning_body": "These rules are enforced the next time each device connects. Devices connected now are unaffected until they reconnect.", "modal_assign_user_device_ip_title": "Device IP settings", "modal_assign_user_device_ip_card_title": "{deviceName} IP settings", "modal_assign_user_device_ip_assignment_description": "You can change the IP address for this device separately in each location/network one-by-one.", From 91d9ff01497b4feb82d8256f4e91a2ae56fbc781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Wed, 5 Aug 2026 15:40:13 +0200 Subject: [PATCH 02/19] add shared helper for triggering a warning modal --- .../EditLocationPage/EditLocationPage.tsx | 87 +++++++++++++---- web/src/shared/utils/postureWarning.ts | 96 +++++++++++++++++++ 2 files changed, 166 insertions(+), 17 deletions(-) create mode 100644 web/src/shared/utils/postureWarning.ts diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index e8b85640f..76c3cae6e 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -43,6 +43,7 @@ import { canUseEnterpriseFeature, } from '../../shared/utils/license'; import { smallestNetworkCapacity } from '../../shared/utils/network'; +import { openPostureAssignmentWarning } from '../../shared/utils/postureWarning'; import { Validate } from '../../shared/validate'; import postureCheckShield from './assets/posture_check_shield.png'; import { getPostureChecksSectionState } from './postureChecksSection'; @@ -472,17 +473,20 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { }, }); - const { mutate: setLocationPostures, isPending: isUpdatingLocationPostures } = - useMutation({ - mutationFn: (data: { postures: number[] }) => - api.devicePosture.setLocationPostures(location.id, data), - meta: { - invalidate: [['device-posture'], ['network'], ['activity-log']], - }, - onError: () => { - Snackbar.error(m.location_posture_checks_update_failed()); - }, - }); + const { + mutate: setLocationPostures, + mutateAsync: setLocationPosturesAsync, + isPending: isUpdatingLocationPostures, + } = useMutation({ + mutationFn: (data: { postures: number[] }) => + api.devicePosture.setLocationPostures(location.id, data), + meta: { + invalidate: [['device-posture'], ['network'], ['activity-log']], + }, + onError: () => { + Snackbar.error(m.location_posture_checks_update_failed()); + }, + }); const openPostureChecksSelection = () => { useSelectionModal.setState({ @@ -500,8 +504,28 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { selected: new Set(location.posture_checks), visibleItemsLimit: 4, onSubmit: (values) => { - setLocationPostures({ - postures: values.filter((value): value is number => typeof value === 'number'), + const newPostureIds = values.filter( + (value): value is number => typeof value === 'number', + ); + const currentIds = location.posture_checks ?? []; + + const addedIds = newPostureIds.filter((id) => !currentIds.includes(id)); + const removedIds = currentIds.filter((id) => !newPostureIds.includes(id)); + + if (addedIds.length === 0 && removedIds.length === 0) return; + + const nameById = new Map(postureChecks.map((p) => [p.id, p.name])); + const addedNames = addedIds.map((id) => nameById.get(id) ?? String(id)); + const removedNames = removedIds.map((id) => nameById.get(id) ?? String(id)); + + openPostureAssignmentWarning({ + kind: 'location', + added: addedNames, + removed: removedNames, + actionPromise: () => setLocationPosturesAsync({ postures: newPostureIds }), + onError: () => { + Snackbar.error(m.location_posture_checks_update_failed()); + }, }); }, }); @@ -995,10 +1019,39 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { counterText={() => ''} disabled={isServiceLocation} onSelectionChange={(values) => { - setLocationPostures({ - postures: values.filter( - (value): value is number => typeof value === 'number', - ), + const newPostureIds = values.filter( + (value): value is number => typeof value === 'number', + ); + const currentIds = location.posture_checks ?? []; + + const addedIds = newPostureIds.filter( + (id) => !currentIds.includes(id), + ); + const removedIds = currentIds.filter( + (id) => !newPostureIds.includes(id), + ); + + if (addedIds.length === 0 && removedIds.length === 0) return; + + const nameById = new Map( + postureChecks.map((p) => [p.id, p.name]), + ); + const addedNames = addedIds.map( + (id) => nameById.get(id) ?? String(id), + ); + const removedNames = removedIds.map( + (id) => nameById.get(id) ?? String(id), + ); + + openPostureAssignmentWarning({ + kind: 'location', + added: addedNames, + removed: removedNames, + actionPromise: () => + setLocationPosturesAsync({ postures: newPostureIds }), + onError: () => { + Snackbar.error(m.location_posture_checks_update_failed()); + }, }); }} onToggleChange={() => {}} diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts new file mode 100644 index 000000000..ccc9f7148 --- /dev/null +++ b/web/src/shared/utils/postureWarning.ts @@ -0,0 +1,96 @@ +import type { QueryKey } from '@tanstack/react-query'; +import type { WebErrorCode } from '../../api/types'; +import { openModal } from '../../hooks/modalControls/modalsSubjects'; +import { ModalName } from '../../hooks/modalControls/modalTypes'; +import { m } from '../../paraglide/messages'; + +type PostureWarningKind = 'location' | 'postures'; + +type BuildPostureWarningChangesArgs = { + added: string[]; + removed: string[]; +}; + +/** + * Builds the `{changes}` markdown fragment listing added and removed items + * in two labelled groups, sorted alphabetically. Returns `null` when both + * sets are empty (no modal needed). + */ +export const buildPostureWarningChanges = ({ + added, + removed, +}: BuildPostureWarningChangesArgs): string | null => { + const sortedAdded = [...added].sort((a, b) => a.localeCompare(b)); + const sortedRemoved = [...removed].sort((a, b) => a.localeCompare(b)); + + const parts: string[] = []; + + if (sortedAdded.length > 0) { + parts.push( + `**${m.modal_posture_assignment_warning_added()}**\n${sortedAdded + .map((name) => `- ${name}`) + .join('\n')}`, + ); + } + + if (sortedRemoved.length > 0) { + parts.push( + `**${m.modal_posture_assignment_warning_removed()}**\n${sortedRemoved + .map((name) => `- ${name}`) + .join('\n')}`, + ); + } + + if (parts.length === 0) return null; + return parts.join('\n\n'); +}; + +type OpenPostureAssignmentWarningArgs = { + kind: PostureWarningKind; + added: string[]; + removed: string[]; + extraBody?: string; + actionPromise: () => Promise; + invalidateKeys?: QueryKey[]; + onSuccess?: (result: unknown) => void; + onError?: (message: string, code?: WebErrorCode) => void; +}; + +/** + * Opens a ConfirmAction modal warning about posture assignment changes. + * Does nothing when both added and removed sets are empty. + */ +export const openPostureAssignmentWarning = ({ + kind, + added, + removed, + extraBody, + actionPromise, + invalidateKeys, + onSuccess, + onError, +}: OpenPostureAssignmentWarningArgs) => { + const changes = buildPostureWarningChanges({ added, removed }); + + if (changes === null) return; + + const bodyKey = + kind === 'location' + ? m.modal_posture_assignment_warning_body_location({ changes }) + : m.modal_posture_assignment_warning_body_postures({ changes }); + + const contentMd = extraBody ? `${bodyKey}\n\n${extraBody}` : bodyKey; + + openModal(ModalName.ConfirmAction, { + title: m.modal_posture_assignment_warning_title(), + contentMd, + actionPromise, + invalidateKeys, + submitProps: { + text: m.controls_save_changes_anyway(), + variant: 'critical', + }, + onSuccess, + onError, + }); +}; From afa3643e78092e3bed7b0ea9b0960df52027155a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Wed, 5 Aug 2026 15:42:38 +0200 Subject: [PATCH 03/19] table & drawer warnings --- .../PostureCheckDrawer/PostureCheckDrawer.tsx | 3 ++- .../PostureChecksPage/PostureChecksTable.tsx | 5 +++- .../PostureChecksPage/postureCheckMenu.ts | 25 ++++++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx b/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx index ef48da7c2..88c65e99c 100644 --- a/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx +++ b/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx @@ -30,7 +30,7 @@ const PostureCheckDrawerContent = ({ row, onClose }: ContentProps) => { const { data: postureCheck } = useSuspenseQuery(getDevicePostureQueryOptions(row.id)); const { data: locations } = useSuspenseQuery(getLocationsQueryOptions); - const { mutate: assignLocations } = useMutation({ + const { mutate: assignLocations, mutateAsync: assignLocationsAsync } = useMutation({ mutationFn: (locationIds: number[]) => api.devicePosture.setLocationsForDevicePosture(row.id, locationIds), meta: { @@ -75,6 +75,7 @@ const PostureCheckDrawerContent = ({ row, onClose }: ContentProps) => { locationOptions, navigate, assignLocations, + assignLocationsAsync, onAfterEdit: onClose, onAfterDelete: onClose, duplicatePosture: () => duplicatePosture(row.id), diff --git a/web/src/pages/PostureChecksPage/PostureChecksTable.tsx b/web/src/pages/PostureChecksPage/PostureChecksTable.tsx index fed710127..62974668b 100644 --- a/web/src/pages/PostureChecksPage/PostureChecksTable.tsx +++ b/web/src/pages/PostureChecksPage/PostureChecksTable.tsx @@ -63,7 +63,7 @@ export const PostureChecksTable = ({ () => buildFilteredLocationOptions(locations), [locations], ); - const { mutate: assignLocations } = useMutation({ + const { mutate: assignLocations, mutateAsync: assignLocationsAsync } = useMutation({ mutationFn: ({ postureCheckId, locations, @@ -235,6 +235,8 @@ export const PostureChecksTable = ({ navigate, assignLocations: (locations) => assignLocations({ postureCheckId: row.id, locations }), + assignLocationsAsync: (locations) => + assignLocationsAsync({ postureCheckId: row.id, locations }), duplicatePosture: () => duplicatePosture(row.id), }); @@ -249,6 +251,7 @@ export const PostureChecksTable = ({ navigate, onRowClick, duplicatePosture, + assignLocationsAsync, ], ); diff --git a/web/src/pages/PostureChecksPage/postureCheckMenu.ts b/web/src/pages/PostureChecksPage/postureCheckMenu.ts index 2e11c0c59..05891e720 100644 --- a/web/src/pages/PostureChecksPage/postureCheckMenu.ts +++ b/web/src/pages/PostureChecksPage/postureCheckMenu.ts @@ -7,6 +7,7 @@ import type { MenuItemsGroup } from '../../shared/defguard-ui/components/Menu/ty import { Snackbar } from '../../shared/defguard-ui/providers/snackbar/snackbar'; import { openModal } from '../../shared/hooks/modalControls/modalsSubjects'; import { ModalName } from '../../shared/hooks/modalControls/modalTypes'; +import { openPostureAssignmentWarning } from '../../shared/utils/postureWarning'; import { getDeletePostureCheckModalData, type PostureCheckRow } from './postureChecks'; type LocationOption = SelectionOption; @@ -16,6 +17,7 @@ type BuildPostureCheckMenuArgs = { locationOptions: LocationOption[]; navigate: ReturnType; assignLocations: (locationIds: number[]) => void; + assignLocationsAsync: (locationIds: number[]) => Promise; duplicatePosture: () => void; onAfterEdit?: () => void; onAfterDelete?: () => void; @@ -26,6 +28,7 @@ export const buildPostureCheckMenuItems = ({ locationOptions, navigate, assignLocations, + assignLocationsAsync, duplicatePosture, onAfterEdit, onAfterDelete, @@ -58,7 +61,27 @@ export const buildPostureCheckMenuItems = ({ options: locationOptions, selected: new Set(row.locations), onSubmit: (selected) => { - assignLocations(selected as number[]); + const newLocationIds = selected as number[]; + const currentIds = row.locations; + + const addedIds = newLocationIds.filter((id) => !currentIds.includes(id)); + const removedIds = currentIds.filter((id) => !newLocationIds.includes(id)); + + if (addedIds.length === 0 && removedIds.length === 0) return; + + const nameById = new Map(locationOptions.map((loc) => [loc.id, loc.label])); + const addedNames = addedIds.map((id) => nameById.get(id) ?? String(id)); + const removedNames = removedIds.map((id) => nameById.get(id) ?? String(id)); + + openPostureAssignmentWarning({ + kind: 'postures', + added: addedNames, + removed: removedNames, + actionPromise: () => assignLocationsAsync(newLocationIds), + onError: () => { + Snackbar.error(m.modal_assign_posture_check_locations_error()); + }, + }); }, }); }, From a32b357c60e68c7e8a6ea1066affcad6fad6af79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 07:16:56 +0200 Subject: [PATCH 04/19] edit posture form warning --- .../EditPostureCheckPage.tsx | 96 ++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index 66a877444..dd97afbed 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -35,6 +35,7 @@ import { getDevicePostureVersionMetadataQueryOptions, getLocationsQueryOptions, } from '../../shared/query'; +import { openPostureAssignmentWarning } from '../../shared/utils/postureWarning'; import { buildAddPostureCheckRequest } from '../AddPostureCheckWizardPage/payload'; import { getPostureCheckVersionValues, @@ -115,6 +116,97 @@ const EditPostureCheckForm = ({ [defaults, values], ); + const rulesChanged = useMemo(() => { + const normalize = (v: EditPostureCheckFormValues) => + JSON.stringify({ + configuredOperatingSystems: [...v.configuredOperatingSystems].sort(), + operatingSystemState: Object.fromEntries( + Object.entries(v.operatingSystemState).map(([os, state]) => [ + os, + { + ...state, + conditions: [...state.conditions].sort(), + }, + ]), + ), + minimumDesktopClientVersion: v.minimumDesktopClientVersion, + minimumMobileClientVersion: v.minimumMobileClientVersion, + allowPrereleaseClient: v.allowPrereleaseClient, + }); + return normalize(values) !== normalize(defaults); + }, [values, defaults]); + + const locationsChanged = useMemo(() => { + if (values.locations.size !== defaults.locations.size) return true; + for (const id of values.locations) { + if (!defaults.locations.has(id)) return true; + } + return false; + }, [values.locations, defaults.locations]); + + const handleSubmit = () => { + if (rulesChanged || locationsChanged) { + const locationAddedIds: number[] = []; + const locationRemovedIds: number[] = []; + + if (locationsChanged) { + for (const id of values.locations) { + if (!defaults.locations.has(id)) locationAddedIds.push(id); + } + for (const id of defaults.locations) { + if (!values.locations.has(id)) locationRemovedIds.push(id); + } + } + + const nameById = new Map(locationOptions.map((loc) => [loc.id, loc.label])); + const addedNames = locationAddedIds.map((id) => nameById.get(id) ?? String(id)); + const removedNames = locationRemovedIds.map((id) => nameById.get(id) ?? String(id)); + + if (rulesChanged && locationsChanged) { + openPostureAssignmentWarning({ + kind: 'postures', + added: addedNames, + removed: removedNames, + extraBody: m.modal_posture_rules_warning_body(), + actionPromise: () => saveMutation.mutateAsync(values), + onError: () => { + Snackbar.error(m.posture_checks_edit_save_failed()); + }, + }); + return; + } + + if (locationsChanged) { + openPostureAssignmentWarning({ + kind: 'postures', + added: addedNames, + removed: removedNames, + actionPromise: () => saveMutation.mutateAsync(values), + onError: () => { + Snackbar.error(m.posture_checks_edit_save_failed()); + }, + }); + return; + } + + openModal(ModalName.ConfirmAction, { + title: m.modal_posture_assignment_warning_title(), + contentMd: m.modal_posture_rules_warning_body(), + actionPromise: () => saveMutation.mutateAsync(values), + submitProps: { + text: m.controls_save_changes_anyway(), + variant: 'critical', + }, + onError: () => { + Snackbar.error(m.posture_checks_edit_save_failed()); + }, + }); + return; + } + + void saveMutation.mutateAsync(values); + }; + const updateValues = ( updater: (current: EditPostureCheckFormValues) => EditPostureCheckFormValues, ) => { @@ -128,7 +220,7 @@ const EditPostureCheckForm = ({
{ event.preventDefault(); - void saveMutation.mutateAsync(values); + handleSubmit(); }} > @@ -187,7 +279,7 @@ const EditPostureCheckForm = ({ disabled: saveDisabled, loading: saveMutation.isPending, onClick: () => { - void saveMutation.mutateAsync(values); + handleSubmit(); }, }} /> From 6bc41db22aa85d7056369eab0abbf9bb16c031af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 07:20:16 +0200 Subject: [PATCH 05/19] remove duplicate translation --- web/messages/en/postures.json | 2 -- .../EditPostureCheckPage/EditPostureCheckPage.tsx | 14 +++++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/web/messages/en/postures.json b/web/messages/en/postures.json index 7f143002a..248d02cd7 100644 --- a/web/messages/en/postures.json +++ b/web/messages/en/postures.json @@ -57,8 +57,6 @@ "posture_checks_edit_defguard": "Defguard", "posture_checks_edit_locations": "Assigned locations", "posture_checks_edit_defguard_note": "\u201cDefguard versions\u201d includes major releases as well as all subsequent patch updates with fixes and improvements.", - "posture_checks_edit_delete_title": "Delete posture check", - "posture_checks_edit_delete_body": "Are you sure you want to delete posture check **{name}**? This action cannot be undone.", "posture_checks_edit_save_success": "Posture check saved", "posture_checks_edit_save_failed": "Failed to save posture check", "posture_checks_edit_delete_success": "Posture check deleted", diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index dd97afbed..bbd34cef2 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -37,6 +37,7 @@ import { } from '../../shared/query'; import { openPostureAssignmentWarning } from '../../shared/utils/postureWarning'; import { buildAddPostureCheckRequest } from '../AddPostureCheckWizardPage/payload'; +import { getDeletePostureCheckModalData } from '../PostureChecksPage/postureChecks'; import { getPostureCheckVersionValues, type PostureCheckVersionValues, @@ -253,12 +254,15 @@ const EditPostureCheckForm = ({ text: m.controls_delete(), disabled: saveMutation.isPending, onClick: () => { + const assignedLocationNames = locationOptions + .filter((loc) => postureCheck.locations.includes(loc.id)) + .map((loc) => loc.label); + openModal(ModalName.ConfirmAction, { - title: m.posture_checks_edit_delete_title(), - contentMd: m.posture_checks_edit_delete_body({ name: postureCheck.name }), - actionPromise: () => api.devicePosture.deleteDevicePosture(postureCheck.id), - invalidateKeys: [['device-posture'], ['network'], ['activity-log']], - submitProps: { text: m.controls_delete(), variant: 'critical' }, + ...getDeletePostureCheckModalData( + { id: postureCheck.id, name: postureCheck.name }, + assignedLocationNames, + ), onSuccess: () => { Snackbar.default(m.posture_checks_edit_delete_success()); navigate({ to: '/acl/posture-checks', replace: true }); From be1f6842dd6065ef5ccfa5318068fa37fd30bfe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 07:22:53 +0200 Subject: [PATCH 06/19] remove unused --- .../EditLocationPage/EditLocationPage.tsx | 25 ++++++++----------- .../PostureCheckDrawer/PostureCheckDrawer.tsx | 3 +-- .../PostureChecksPage/PostureChecksTable.tsx | 5 +--- .../PostureChecksPage/postureCheckMenu.ts | 2 -- web/src/shared/utils/postureWarning.ts | 6 ++--- 5 files changed, 16 insertions(+), 25 deletions(-) diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index 76c3cae6e..18fd12302 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -473,20 +473,17 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { }, }); - const { - mutate: setLocationPostures, - mutateAsync: setLocationPosturesAsync, - isPending: isUpdatingLocationPostures, - } = useMutation({ - mutationFn: (data: { postures: number[] }) => - api.devicePosture.setLocationPostures(location.id, data), - meta: { - invalidate: [['device-posture'], ['network'], ['activity-log']], - }, - onError: () => { - Snackbar.error(m.location_posture_checks_update_failed()); - }, - }); + const { mutateAsync: setLocationPosturesAsync, isPending: isUpdatingLocationPostures } = + useMutation({ + mutationFn: (data: { postures: number[] }) => + api.devicePosture.setLocationPostures(location.id, data), + meta: { + invalidate: [['device-posture'], ['network'], ['activity-log']], + }, + onError: () => { + Snackbar.error(m.location_posture_checks_update_failed()); + }, + }); const openPostureChecksSelection = () => { useSelectionModal.setState({ diff --git a/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx b/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx index 88c65e99c..41b6a22e6 100644 --- a/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx +++ b/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx @@ -30,7 +30,7 @@ const PostureCheckDrawerContent = ({ row, onClose }: ContentProps) => { const { data: postureCheck } = useSuspenseQuery(getDevicePostureQueryOptions(row.id)); const { data: locations } = useSuspenseQuery(getLocationsQueryOptions); - const { mutate: assignLocations, mutateAsync: assignLocationsAsync } = useMutation({ + const { mutateAsync: assignLocationsAsync } = useMutation({ mutationFn: (locationIds: number[]) => api.devicePosture.setLocationsForDevicePosture(row.id, locationIds), meta: { @@ -74,7 +74,6 @@ const PostureCheckDrawerContent = ({ row, onClose }: ContentProps) => { row, locationOptions, navigate, - assignLocations, assignLocationsAsync, onAfterEdit: onClose, onAfterDelete: onClose, diff --git a/web/src/pages/PostureChecksPage/PostureChecksTable.tsx b/web/src/pages/PostureChecksPage/PostureChecksTable.tsx index 62974668b..a2ba05b3c 100644 --- a/web/src/pages/PostureChecksPage/PostureChecksTable.tsx +++ b/web/src/pages/PostureChecksPage/PostureChecksTable.tsx @@ -63,7 +63,7 @@ export const PostureChecksTable = ({ () => buildFilteredLocationOptions(locations), [locations], ); - const { mutate: assignLocations, mutateAsync: assignLocationsAsync } = useMutation({ + const { mutateAsync: assignLocationsAsync } = useMutation({ mutationFn: ({ postureCheckId, locations, @@ -233,8 +233,6 @@ export const PostureChecksTable = ({ row, locationOptions, navigate, - assignLocations: (locations) => - assignLocations({ postureCheckId: row.id, locations }), assignLocationsAsync: (locations) => assignLocationsAsync({ postureCheckId: row.id, locations }), duplicatePosture: () => duplicatePosture(row.id), @@ -245,7 +243,6 @@ export const PostureChecksTable = ({ }), ], [ - assignLocations, columnFilterOptions, locationOptions, navigate, diff --git a/web/src/pages/PostureChecksPage/postureCheckMenu.ts b/web/src/pages/PostureChecksPage/postureCheckMenu.ts index 05891e720..9b1447c38 100644 --- a/web/src/pages/PostureChecksPage/postureCheckMenu.ts +++ b/web/src/pages/PostureChecksPage/postureCheckMenu.ts @@ -16,7 +16,6 @@ type BuildPostureCheckMenuArgs = { row: PostureCheckRow; locationOptions: LocationOption[]; navigate: ReturnType; - assignLocations: (locationIds: number[]) => void; assignLocationsAsync: (locationIds: number[]) => Promise; duplicatePosture: () => void; onAfterEdit?: () => void; @@ -27,7 +26,6 @@ export const buildPostureCheckMenuItems = ({ row, locationOptions, navigate, - assignLocations, assignLocationsAsync, duplicatePosture, onAfterEdit, diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index ccc9f7148..0c613bec4 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -1,8 +1,8 @@ import type { QueryKey } from '@tanstack/react-query'; -import type { WebErrorCode } from '../../api/types'; -import { openModal } from '../../hooks/modalControls/modalsSubjects'; -import { ModalName } from '../../hooks/modalControls/modalTypes'; import { m } from '../../paraglide/messages'; +import type { WebErrorCode } from '../api/types'; +import { openModal } from '../hooks/modalControls/modalsSubjects'; +import { ModalName } from '../hooks/modalControls/modalTypes'; type PostureWarningKind = 'location' | 'postures'; From 8f90163618fb65acd920d8239473840ef755e3e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 07:33:17 +0200 Subject: [PATCH 07/19] don't trigger modal when editing a posture not assigned to location --- web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index bbd34cef2..5fb25b4c1 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -146,6 +146,11 @@ const EditPostureCheckForm = ({ }, [values.locations, defaults.locations]); const handleSubmit = () => { + if (defaults.locations.size === 0 && !locationsChanged) { + void saveMutation.mutateAsync(values); + return; + } + if (rulesChanged || locationsChanged) { const locationAddedIds: number[] = []; const locationRemovedIds: number[] = []; From a48c85d773cebe81aa6da2e3cb3f89235f727d9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 09:52:07 +0200 Subject: [PATCH 08/19] add separate exports for different use-cases --- web/src/shared/utils/postureWarning.ts | 127 +++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index 0c613bec4..da112cb7b 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -94,3 +94,130 @@ export const openPostureAssignmentWarning = ({ onError, }); }; + +type Option = { readonly id: number; readonly label: string }; + +/** Escape markdown control characters so admin-supplied labels render as literal text. */ +const escapeMarkdown = (s: string) => s.replace(/[\\`*_{}[\]()#+\-.!>|~]/g, '\\$&'); + +type ConfirmSelectionChangeArgs = { + current: Iterable; + next: Iterable; + options: readonly Option[]; + actionPromise: () => Promise; + deferredEnforcement?: boolean; + bodyMessage: (args: { changes: string }) => string; +}; + +/** + * Diffs id sets, resolves labels, escapes markdown, composes the four + * states (diff only, deferred-enforcement only, both, neither) and opens + * the ConfirmAction modal. Returns `true` when a modal was opened. + */ +const confirmSelectionChange = ({ + current, + next, + options, + actionPromise, + deferredEnforcement, + bodyMessage, +}: ConfirmSelectionChangeArgs): boolean => { + const currentSet = new Set(current); + const nextSet = new Set(next); + + const addedIds = [...nextSet].filter((id) => !currentSet.has(id)); + const removedIds = [...currentSet].filter((id) => !nextSet.has(id)); + + const labelMap = new Map(options.map((o) => [o.id, o.label])); + + const sortedAdded = addedIds + .map((id) => escapeMarkdown(labelMap.get(id) ?? String(id))) + .sort((a, b) => a.localeCompare(b)); + const sortedRemoved = removedIds + .map((id) => escapeMarkdown(labelMap.get(id) ?? String(id))) + .sort((a, b) => a.localeCompare(b)); + + const parts: string[] = []; + + if (sortedAdded.length > 0) { + parts.push( + `**${m.modal_posture_assignment_warning_added()}**\n\n${sortedAdded + .map((name) => `- ${name}`) + .join('\n')}`, + ); + } + + if (sortedRemoved.length > 0) { + parts.push( + `**${m.modal_posture_assignment_warning_removed()}**\n\n${sortedRemoved + .map((name) => `- ${name}`) + .join('\n')}`, + ); + } + + const changes = parts.length > 0 ? parts.join('\n\n') : null; + const hasDiff = changes !== null; + + if (!hasDiff && !deferredEnforcement) return false; + + let contentMd: string; + if (hasDiff) { + contentMd = deferredEnforcement + ? `${bodyMessage({ changes })}\n\n${m.modal_posture_rules_warning_body()}` + : bodyMessage({ changes }); + } else { + contentMd = m.modal_posture_rules_warning_body(); + } + + openModal(ModalName.ConfirmAction, { + title: m.modal_posture_assignment_warning_title(), + contentMd, + actionPromise, + submitProps: { + text: m.controls_save_changes_anyway(), + variant: 'critical', + }, + }); + + return true; +}; + +/** + * Warn when the set of assigned posture checks on a location changes. + * Ids are posture-check ids; the body warns about active sessions for + * this location. + */ +export const confirmPostureSelectionChange = ( + current: Iterable, + next: Iterable, + options: readonly Option[], + actionPromise: () => Promise, +): boolean => + confirmSelectionChange({ + current, + next, + options, + actionPromise, + bodyMessage: m.modal_posture_assignment_warning_body_location, + }); + +/** + * Warn when the set of assigned locations on a posture check changes. + * When `deferredEnforcement` is true, appends the rules-deferred paragraph + * to the warning body. + */ +export const confirmLocationSelectionChange = ( + current: Iterable, + next: Iterable, + options: readonly Option[], + actionPromise: () => Promise, + deferredEnforcement?: boolean, +): boolean => + confirmSelectionChange({ + current, + next, + options, + actionPromise, + deferredEnforcement, + bodyMessage: m.modal_posture_assignment_warning_body_postures, + }); From 41d16c658175dd10ad5ca53c8a833cf08dddc7ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 10:03:32 +0200 Subject: [PATCH 09/19] migrate location edit page --- .../EditLocationPage/EditLocationPage.tsx | 84 ++++--------------- 1 file changed, 16 insertions(+), 68 deletions(-) diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index 18fd12302..70d6b4a03 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -43,7 +43,7 @@ import { canUseEnterpriseFeature, } from '../../shared/utils/license'; import { smallestNetworkCapacity } from '../../shared/utils/network'; -import { openPostureAssignmentWarning } from '../../shared/utils/postureWarning'; +import { confirmPostureSelectionChange } from '../../shared/utils/postureWarning'; import { Validate } from '../../shared/validate'; import postureCheckShield from './assets/posture_check_shield.png'; import { getPostureChecksSectionState } from './postureChecksSection'; @@ -393,15 +393,12 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { ); const assignedPostureChecks = useMemo(() => { - const labelsById = new Map( - postureChecks.map((postureCheck) => [postureCheck.id, postureCheck.name]), - ); - + const labelByOption = new Map(postureCheckOptions.map((o) => [o.id, o.label])); return location.posture_checks?.map((id) => ({ id, - label: labelsById.get(id) ?? String(id), + label: labelByOption.get(id) ?? String(id), })); - }, [location.posture_checks, postureChecks]); + }, [location.posture_checks, postureCheckOptions]); const serviceLocationLabelContent = useMemo(() => { if (!serviceLocationLocked) return undefined; @@ -485,6 +482,16 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { }, }); + const handlePostureSelection = (values: (string | number)[]) => { + const next = values.filter((v): v is number => typeof v === 'number'); + confirmPostureSelectionChange( + location.posture_checks ?? [], + next, + postureCheckOptions, + () => setLocationPosturesAsync({ postures: next }), + ); + }; + const openPostureChecksSelection = () => { useSelectionModal.setState({ isOpen: true, @@ -500,31 +507,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { searchPlaceholder: m.controls_search(), selected: new Set(location.posture_checks), visibleItemsLimit: 4, - onSubmit: (values) => { - const newPostureIds = values.filter( - (value): value is number => typeof value === 'number', - ); - const currentIds = location.posture_checks ?? []; - - const addedIds = newPostureIds.filter((id) => !currentIds.includes(id)); - const removedIds = currentIds.filter((id) => !newPostureIds.includes(id)); - - if (addedIds.length === 0 && removedIds.length === 0) return; - - const nameById = new Map(postureChecks.map((p) => [p.id, p.name])); - const addedNames = addedIds.map((id) => nameById.get(id) ?? String(id)); - const removedNames = removedIds.map((id) => nameById.get(id) ?? String(id)); - - openPostureAssignmentWarning({ - kind: 'location', - added: addedNames, - removed: removedNames, - actionPromise: () => setLocationPosturesAsync({ postures: newPostureIds }), - onError: () => { - Snackbar.error(m.location_posture_checks_update_failed()); - }, - }); - }, + onSubmit: handlePostureSelection, }); }; @@ -1015,42 +998,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { toggleValue={false} counterText={() => ''} disabled={isServiceLocation} - onSelectionChange={(values) => { - const newPostureIds = values.filter( - (value): value is number => typeof value === 'number', - ); - const currentIds = location.posture_checks ?? []; - - const addedIds = newPostureIds.filter( - (id) => !currentIds.includes(id), - ); - const removedIds = currentIds.filter( - (id) => !newPostureIds.includes(id), - ); - - if (addedIds.length === 0 && removedIds.length === 0) return; - - const nameById = new Map( - postureChecks.map((p) => [p.id, p.name]), - ); - const addedNames = addedIds.map( - (id) => nameById.get(id) ?? String(id), - ); - const removedNames = removedIds.map( - (id) => nameById.get(id) ?? String(id), - ); - - openPostureAssignmentWarning({ - kind: 'location', - added: addedNames, - removed: removedNames, - actionPromise: () => - setLocationPosturesAsync({ postures: newPostureIds }), - onError: () => { - Snackbar.error(m.location_posture_checks_update_failed()); - }, - }); - }} + onSelectionChange={handlePostureSelection} onToggleChange={() => {}} selectionCustomItemRender={renderPostureCheckSelectionItem} selectionModalProps={{ From e475e1dbadd700a916ab526a9971cf6407f9577b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 10:10:11 +0200 Subject: [PATCH 10/19] migrate table and drawer --- .../PostureCheckDrawer/PostureCheckDrawer.tsx | 2 +- .../PostureChecksPage/PostureChecksTable.tsx | 2 +- .../PostureChecksPage/postureCheckMenu.ts | 31 +++++-------------- 3 files changed, 9 insertions(+), 26 deletions(-) diff --git a/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx b/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx index 41b6a22e6..3264f0a4d 100644 --- a/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx +++ b/web/src/pages/PostureChecksPage/PostureCheckDrawer/PostureCheckDrawer.tsx @@ -74,7 +74,7 @@ const PostureCheckDrawerContent = ({ row, onClose }: ContentProps) => { row, locationOptions, navigate, - assignLocationsAsync, + assignLocations: assignLocationsAsync, onAfterEdit: onClose, onAfterDelete: onClose, duplicatePosture: () => duplicatePosture(row.id), diff --git a/web/src/pages/PostureChecksPage/PostureChecksTable.tsx b/web/src/pages/PostureChecksPage/PostureChecksTable.tsx index a2ba05b3c..eec3fc717 100644 --- a/web/src/pages/PostureChecksPage/PostureChecksTable.tsx +++ b/web/src/pages/PostureChecksPage/PostureChecksTable.tsx @@ -233,7 +233,7 @@ export const PostureChecksTable = ({ row, locationOptions, navigate, - assignLocationsAsync: (locations) => + assignLocations: (locations) => assignLocationsAsync({ postureCheckId: row.id, locations }), duplicatePosture: () => duplicatePosture(row.id), }); diff --git a/web/src/pages/PostureChecksPage/postureCheckMenu.ts b/web/src/pages/PostureChecksPage/postureCheckMenu.ts index 9b1447c38..e91c0e733 100644 --- a/web/src/pages/PostureChecksPage/postureCheckMenu.ts +++ b/web/src/pages/PostureChecksPage/postureCheckMenu.ts @@ -7,7 +7,7 @@ import type { MenuItemsGroup } from '../../shared/defguard-ui/components/Menu/ty import { Snackbar } from '../../shared/defguard-ui/providers/snackbar/snackbar'; import { openModal } from '../../shared/hooks/modalControls/modalsSubjects'; import { ModalName } from '../../shared/hooks/modalControls/modalTypes'; -import { openPostureAssignmentWarning } from '../../shared/utils/postureWarning'; +import { confirmLocationSelectionChange } from '../../shared/utils/postureWarning'; import { getDeletePostureCheckModalData, type PostureCheckRow } from './postureChecks'; type LocationOption = SelectionOption; @@ -16,7 +16,7 @@ type BuildPostureCheckMenuArgs = { row: PostureCheckRow; locationOptions: LocationOption[]; navigate: ReturnType; - assignLocationsAsync: (locationIds: number[]) => Promise; + assignLocations: (locationIds: number[]) => Promise; duplicatePosture: () => void; onAfterEdit?: () => void; onAfterDelete?: () => void; @@ -26,7 +26,7 @@ export const buildPostureCheckMenuItems = ({ row, locationOptions, navigate, - assignLocationsAsync, + assignLocations, duplicatePosture, onAfterEdit, onAfterDelete, @@ -59,27 +59,10 @@ export const buildPostureCheckMenuItems = ({ options: locationOptions, selected: new Set(row.locations), onSubmit: (selected) => { - const newLocationIds = selected as number[]; - const currentIds = row.locations; - - const addedIds = newLocationIds.filter((id) => !currentIds.includes(id)); - const removedIds = currentIds.filter((id) => !newLocationIds.includes(id)); - - if (addedIds.length === 0 && removedIds.length === 0) return; - - const nameById = new Map(locationOptions.map((loc) => [loc.id, loc.label])); - const addedNames = addedIds.map((id) => nameById.get(id) ?? String(id)); - const removedNames = removedIds.map((id) => nameById.get(id) ?? String(id)); - - openPostureAssignmentWarning({ - kind: 'postures', - added: addedNames, - removed: removedNames, - actionPromise: () => assignLocationsAsync(newLocationIds), - onError: () => { - Snackbar.error(m.modal_assign_posture_check_locations_error()); - }, - }); + const next = selected as number[]; + confirmLocationSelectionChange(row.locations, next, locationOptions, () => + assignLocations(next), + ); }, }); }, From 6cfcd07a2d4470e77d429ea09464c80272aad7f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 10:25:01 +0200 Subject: [PATCH 11/19] migrate posture edit --- .../EditPostureCheckPage.tsx | 74 +++------------ web/src/shared/utils/postureWarning.ts | 93 ------------------- 2 files changed, 12 insertions(+), 155 deletions(-) diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index 5fb25b4c1..f9332e8f4 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -35,7 +35,7 @@ import { getDevicePostureVersionMetadataQueryOptions, getLocationsQueryOptions, } from '../../shared/query'; -import { openPostureAssignmentWarning } from '../../shared/utils/postureWarning'; +import { confirmLocationSelectionChange } from '../../shared/utils/postureWarning'; import { buildAddPostureCheckRequest } from '../AddPostureCheckWizardPage/payload'; import { getDeletePostureCheckModalData } from '../PostureChecksPage/postureChecks'; import { @@ -146,71 +146,21 @@ const EditPostureCheckForm = ({ }, [values.locations, defaults.locations]); const handleSubmit = () => { - if (defaults.locations.size === 0 && !locationsChanged) { - void saveMutation.mutateAsync(values); - return; - } - if (rulesChanged || locationsChanged) { - const locationAddedIds: number[] = []; - const locationRemovedIds: number[] = []; - - if (locationsChanged) { - for (const id of values.locations) { - if (!defaults.locations.has(id)) locationAddedIds.push(id); - } - for (const id of defaults.locations) { - if (!values.locations.has(id)) locationRemovedIds.push(id); - } - } - - const nameById = new Map(locationOptions.map((loc) => [loc.id, loc.label])); - const addedNames = locationAddedIds.map((id) => nameById.get(id) ?? String(id)); - const removedNames = locationRemovedIds.map((id) => nameById.get(id) ?? String(id)); - - if (rulesChanged && locationsChanged) { - openPostureAssignmentWarning({ - kind: 'postures', - added: addedNames, - removed: removedNames, - extraBody: m.modal_posture_rules_warning_body(), - actionPromise: () => saveMutation.mutateAsync(values), - onError: () => { - Snackbar.error(m.posture_checks_edit_save_failed()); - }, - }); + const deferred = rulesChanged && defaults.locations.size > 0; + if ( + confirmLocationSelectionChange( + [...defaults.locations], + [...values.locations], + locationOptions, + () => saveMutation.mutateAsync(values), + deferred, + ) + ) { return; } - - if (locationsChanged) { - openPostureAssignmentWarning({ - kind: 'postures', - added: addedNames, - removed: removedNames, - actionPromise: () => saveMutation.mutateAsync(values), - onError: () => { - Snackbar.error(m.posture_checks_edit_save_failed()); - }, - }); - return; - } - - openModal(ModalName.ConfirmAction, { - title: m.modal_posture_assignment_warning_title(), - contentMd: m.modal_posture_rules_warning_body(), - actionPromise: () => saveMutation.mutateAsync(values), - submitProps: { - text: m.controls_save_changes_anyway(), - variant: 'critical', - }, - onError: () => { - Snackbar.error(m.posture_checks_edit_save_failed()); - }, - }); - return; } - - void saveMutation.mutateAsync(values); + saveMutation.mutate(values); }; const updateValues = ( diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index da112cb7b..aa0b859d4 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -1,100 +1,7 @@ -import type { QueryKey } from '@tanstack/react-query'; import { m } from '../../paraglide/messages'; -import type { WebErrorCode } from '../api/types'; import { openModal } from '../hooks/modalControls/modalsSubjects'; import { ModalName } from '../hooks/modalControls/modalTypes'; -type PostureWarningKind = 'location' | 'postures'; - -type BuildPostureWarningChangesArgs = { - added: string[]; - removed: string[]; -}; - -/** - * Builds the `{changes}` markdown fragment listing added and removed items - * in two labelled groups, sorted alphabetically. Returns `null` when both - * sets are empty (no modal needed). - */ -export const buildPostureWarningChanges = ({ - added, - removed, -}: BuildPostureWarningChangesArgs): string | null => { - const sortedAdded = [...added].sort((a, b) => a.localeCompare(b)); - const sortedRemoved = [...removed].sort((a, b) => a.localeCompare(b)); - - const parts: string[] = []; - - if (sortedAdded.length > 0) { - parts.push( - `**${m.modal_posture_assignment_warning_added()}**\n${sortedAdded - .map((name) => `- ${name}`) - .join('\n')}`, - ); - } - - if (sortedRemoved.length > 0) { - parts.push( - `**${m.modal_posture_assignment_warning_removed()}**\n${sortedRemoved - .map((name) => `- ${name}`) - .join('\n')}`, - ); - } - - if (parts.length === 0) return null; - return parts.join('\n\n'); -}; - -type OpenPostureAssignmentWarningArgs = { - kind: PostureWarningKind; - added: string[]; - removed: string[]; - extraBody?: string; - actionPromise: () => Promise; - invalidateKeys?: QueryKey[]; - onSuccess?: (result: unknown) => void; - onError?: (message: string, code?: WebErrorCode) => void; -}; - -/** - * Opens a ConfirmAction modal warning about posture assignment changes. - * Does nothing when both added and removed sets are empty. - */ -export const openPostureAssignmentWarning = ({ - kind, - added, - removed, - extraBody, - actionPromise, - invalidateKeys, - onSuccess, - onError, -}: OpenPostureAssignmentWarningArgs) => { - const changes = buildPostureWarningChanges({ added, removed }); - - if (changes === null) return; - - const bodyKey = - kind === 'location' - ? m.modal_posture_assignment_warning_body_location({ changes }) - : m.modal_posture_assignment_warning_body_postures({ changes }); - - const contentMd = extraBody ? `${bodyKey}\n\n${extraBody}` : bodyKey; - - openModal(ModalName.ConfirmAction, { - title: m.modal_posture_assignment_warning_title(), - contentMd, - actionPromise, - invalidateKeys, - submitProps: { - text: m.controls_save_changes_anyway(), - variant: 'critical', - }, - onSuccess, - onError, - }); -}; - type Option = { readonly id: number; readonly label: string }; /** Escape markdown control characters so admin-supplied labels render as literal text. */ From 96ebb8fc345bb7654daab78ade8e0d1fc12aa73b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 10:27:49 +0200 Subject: [PATCH 12/19] add normalization helper --- .../EditPostureCheckPage.tsx | 26 +++++-------------- web/src/pages/EditPostureCheckPage/form.ts | 23 ++++++++++++++++ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index f9332e8f4..411cff1a6 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -45,6 +45,7 @@ import { import { type EditPostureCheckFormValues, getInitialEditPostureCheckFormValues, + normalizeEditPostureCheckEnforcementFields, normalizeEditPostureCheckFormValues, } from './form'; @@ -117,25 +118,12 @@ const EditPostureCheckForm = ({ [defaults, values], ); - const rulesChanged = useMemo(() => { - const normalize = (v: EditPostureCheckFormValues) => - JSON.stringify({ - configuredOperatingSystems: [...v.configuredOperatingSystems].sort(), - operatingSystemState: Object.fromEntries( - Object.entries(v.operatingSystemState).map(([os, state]) => [ - os, - { - ...state, - conditions: [...state.conditions].sort(), - }, - ]), - ), - minimumDesktopClientVersion: v.minimumDesktopClientVersion, - minimumMobileClientVersion: v.minimumMobileClientVersion, - allowPrereleaseClient: v.allowPrereleaseClient, - }); - return normalize(values) !== normalize(defaults); - }, [values, defaults]); + const rulesChanged = useMemo( + () => + JSON.stringify(normalizeEditPostureCheckEnforcementFields(values)) !== + JSON.stringify(normalizeEditPostureCheckEnforcementFields(defaults)), + [defaults, values], + ); const locationsChanged = useMemo(() => { if (values.locations.size !== defaults.locations.size) return true; diff --git a/web/src/pages/EditPostureCheckPage/form.ts b/web/src/pages/EditPostureCheckPage/form.ts index 3c17ba0a4..800c03c76 100644 --- a/web/src/pages/EditPostureCheckPage/form.ts +++ b/web/src/pages/EditPostureCheckPage/form.ts @@ -139,5 +139,28 @@ export const normalizeEditPostureCheckFormValues = ( values: EditPostureCheckFormValues, ) => ({ ...values, + configuredOperatingSystems: [...values.configuredOperatingSystems].sort(), locations: Array.from(values.locations).sort((left, right) => left - right), + operatingSystemState: Object.fromEntries( + Object.entries(values.operatingSystemState).map(([os, state]) => [ + os, + { ...state, conditions: [...state.conditions].sort() }, + ]), + ), +}); + +/** Projection of enforcement-related fields for `rulesChanged` comparison. */ +export const normalizeEditPostureCheckEnforcementFields = ( + v: EditPostureCheckFormValues, +) => ({ + allowPrereleaseClient: v.allowPrereleaseClient, + configuredOperatingSystems: [...v.configuredOperatingSystems].sort(), + minimumDesktopClientVersion: v.minimumDesktopClientVersion, + minimumMobileClientVersion: v.minimumMobileClientVersion, + operatingSystemState: Object.fromEntries( + Object.entries(v.operatingSystemState).map(([os, state]) => [ + os, + { ...state, conditions: [...state.conditions].sort() }, + ]), + ), }); From 03dacd89d4389bb38992a97d3791e3d6808647e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 10:34:52 +0200 Subject: [PATCH 13/19] add unit tests --- web/tests/posture-check-delete.test.ts | 4 +- web/tests/posture-checks-page.test.ts | 2 +- web/tests/posture-warning.test.ts | 237 +++++++++++++++++++++++++ 3 files changed, 240 insertions(+), 3 deletions(-) create mode 100644 web/tests/posture-warning.test.ts diff --git a/web/tests/posture-check-delete.test.ts b/web/tests/posture-check-delete.test.ts index 982b71896..98fe09428 100644 --- a/web/tests/posture-check-delete.test.ts +++ b/web/tests/posture-check-delete.test.ts @@ -18,7 +18,7 @@ describe('posture check delete confirmation', () => { expect(modalData.title).toBe('Delete posture check'); expect(modalData.contentMd).toBe( - 'Are you sure you want to delete this check? It’s currently used in Warsaw and Berlin. Removing it may change access criteria for users in these locations.', + 'Are you sure you want to delete this check? It’s currently used in Warsaw and Berlin. Removing it may change access criteria for users in these locations and disconnect active VPN client sessions there.', ); expect(modalData.invalidateKeys).toEqual([ ['device-posture'], @@ -45,7 +45,7 @@ describe('posture check delete confirmation', () => { ); expect(modalData.contentMd).toBe( - 'Are you sure you want to delete this check? It’s currently used in Warsaw, Berlin, and Paris. Removing it may change access criteria for users in these locations.', + 'Are you sure you want to delete this check? It’s currently used in Warsaw, Berlin, and Paris. Removing it may change access criteria for users in these locations and disconnect active VPN client sessions there.', ); }); diff --git a/web/tests/posture-checks-page.test.ts b/web/tests/posture-checks-page.test.ts index cba9ecafb..6c6991d05 100644 --- a/web/tests/posture-checks-page.test.ts +++ b/web/tests/posture-checks-page.test.ts @@ -271,7 +271,7 @@ describe('posture checks page helpers', () => { ), ).toEqual({ allowPrereleaseClient: true, - configuredOperatingSystems: ['windows', 'android'], + configuredOperatingSystems: ['android', 'windows'], description: 'Existing policy', locations: [3, 9], minimumDesktopClientVersion: '2.0', diff --git a/web/tests/posture-warning.test.ts b/web/tests/posture-warning.test.ts new file mode 100644 index 000000000..7727ef09e --- /dev/null +++ b/web/tests/posture-warning.test.ts @@ -0,0 +1,237 @@ +import { describe, expect, it, vi } from 'vitest'; +import { openModal } from '../src/shared/hooks/modalControls/modalsSubjects'; + +vi.mock('../src/shared/hooks/modalControls/modalsSubjects', () => ({ + openModal: vi.fn(), +})); + +vi.mock('../src/shared/hooks/modalControls/modalTypes', () => ({ + ModalName: { ConfirmAction: 'confirmAction' }, +})); + +vi.mock('../src/paraglide/messages', () => ({ + m: { + modal_posture_assignment_warning_added: () => 'Added:', + modal_posture_assignment_warning_removed: () => 'Removed:', + modal_posture_assignment_warning_title: () => 'Confirm posture check changes', + modal_posture_assignment_warning_body_location: ({ changes }: { changes: string }) => + `These changes may disconnect active VPN client sessions for this location.\n\n${changes}`, + modal_posture_assignment_warning_body_postures: ({ changes }: { changes: string }) => + `These changes may disconnect active VPN client sessions for the affected locations.\n\n${changes}`, + modal_posture_rules_warning_body: () => + 'These rules are enforced the next time each device connects. Devices connected now are unaffected until they reconnect.', + controls_save_changes_anyway: () => 'Save changes anyway', + }, +})); + +import { + confirmLocationSelectionChange, + confirmPostureSelectionChange, +} from '../src/shared/utils/postureWarning'; + +type Option = { readonly id: number; readonly label: string }; + +const locOptions: Option[] = [ + { id: 1, label: 'Berlin' }, + { id: 2, label: 'Amsterdam' }, + { id: 3, label: 'Zurich' }, +]; + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe('confirmPostureSelectionChange', () => { + it('returns false and opens nothing when the id sets are identical', () => { + const result = confirmPostureSelectionChange( + [1, 2], + [1, 2], + locOptions, + async () => {}, + ); + + expect(result).toBe(false); + expect(openModal).not.toHaveBeenCalled(); + }); + + it('returns false and opens nothing when both sets are empty', () => { + const result = confirmPostureSelectionChange([], [], locOptions, async () => {}); + + expect(result).toBe(false); + expect(openModal).not.toHaveBeenCalled(); + }); + + it('opens a modal with the Added group when items were added', () => { + confirmPostureSelectionChange([1], [1, 2, 3], locOptions, async () => {}); + + expect(openModal).toHaveBeenCalledOnce(); + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + // Added: group sorted alphabetically + expect(contentMd).toContain('**Added:**'); + expect(contentMd).toContain('- Amsterdam'); + expect(contentMd).toContain('- Zurich'); + // Added: items appear in alphabetical order + expect(contentMd.indexOf('Amsterdam')).toBeLessThan(contentMd.indexOf('Zurich')); + expect(contentMd).not.toContain('Removed'); + }); + + it('opens a modal with the Removed group when items were removed', () => { + confirmPostureSelectionChange([1, 2, 3], [1], locOptions, async () => {}); + + expect(openModal).toHaveBeenCalledOnce(); + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('**Removed:**'); + expect(contentMd).toContain('- Amsterdam'); + expect(contentMd).toContain('- Zurich'); + expect(contentMd.indexOf('Amsterdam')).toBeLessThan(contentMd.indexOf('Zurich')); + expect(contentMd).not.toContain('Added'); + }); + + it('opens a modal with both Added and Removed groups when items changed', () => { + confirmPostureSelectionChange([1], [2, 3], locOptions, async () => {}); + + expect(openModal).toHaveBeenCalledOnce(); + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('**Added:**'); + expect(contentMd).toContain('**Removed:**'); + expect(contentMd.indexOf('Added:')).toBeLessThan(contentMd.indexOf('Removed:')); + }); + + it('includes the location-warning body message', () => { + confirmPostureSelectionChange([1], [2], locOptions, async () => {}); + + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('for this location.'); + }); +}); + +describe('confirmLocationSelectionChange', () => { + it('returns false when no diff and no deferredEnforcement', () => { + const result = confirmLocationSelectionChange( + [1, 2], + [1, 2], + locOptions, + async () => {}, + ); + + expect(result).toBe(false); + expect(openModal).not.toHaveBeenCalled(); + }); + + it('includes the postures-warning body message', () => { + confirmLocationSelectionChange([1], [2], locOptions, async () => {}); + + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('for the affected locations.'); + }); + + it('opens rules-only body when deferredEnforcement is true and no location diff', () => { + confirmLocationSelectionChange([1, 2], [1, 2], locOptions, async () => {}, true); + + expect(openModal).toHaveBeenCalledOnce(); + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('These rules are enforced'); + expect(contentMd).not.toContain('Added'); + expect(contentMd).not.toContain('Removed'); + }); + + it('appends rules paragraph after locations diff when deferredEnforcement is true and diff exists', () => { + confirmLocationSelectionChange([1], [2], locOptions, async () => {}, true); + + expect(openModal).toHaveBeenCalledOnce(); + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('for the affected locations.'); + expect(contentMd).toContain('These rules are enforced'); + // rules paragraph appears after the locations body + const locIndex = contentMd.indexOf('for the affected locations.'); + const rulesIndex = contentMd.indexOf('These rules are enforced'); + expect(locIndex).toBeLessThan(rulesIndex); + }); + + it('returns false when deferredEnforcement is false (the falsy default)', () => { + const result = confirmLocationSelectionChange( + [1], + [1], + locOptions, + async () => {}, + false, + ); + + expect(result).toBe(false); + }); +}); + +describe('markdown escaping', () => { + it('escapes markdown control characters in labels', () => { + const spikyOptions: Option[] = [ + { id: 1, label: 'plain' }, + { id: 2, label: '*bold*' }, + { id: 3, label: '[link](url)' }, + { id: 4, label: 'back`tick' }, + ]; + + confirmPostureSelectionChange([1], [2, 3, 4], spikyOptions, async () => {}); + + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + // Asterisks escaped + expect(contentMd).toContain('\\*bold\\*'); + expect(contentMd).not.toMatch(/(? { + const underscoreOptions: Option[] = [ + { id: 1, label: 'old' }, + { id: 2, label: 'os_version_check' }, + ]; + + confirmPostureSelectionChange([1], [2], underscoreOptions, async () => {}); + + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).not.toMatch(/(? { + it('uses String(id) when an id has no matching option', () => { + confirmPostureSelectionChange( + [], + [1, 99], + [{ id: 1, label: 'Known' }], + async () => {}, + ); + + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('- 99'); + expect(contentMd).toContain('- Known'); + }); +}); + +describe('modal structure', () => { + it('opens ConfirmAction with the shared title, critical variant, and actionPromise', () => { + const actionPromise = async () => 'saved'; + confirmPostureSelectionChange([1], [2], locOptions, actionPromise); + + const modalData = vi.mocked(openModal).mock.calls[0][1]; + expect(openModal).toHaveBeenCalledWith( + 'confirmAction', + expect.objectContaining({ + title: 'Confirm posture check changes', + actionPromise, + submitProps: { + text: 'Save changes anyway', + variant: 'critical', + }, + }), + ); + // No stale keys from the old helper + expect(modalData.invalidateKeys).toBeUndefined(); + expect(modalData.onSuccess).toBeUndefined(); + expect(modalData.onError).toBeUndefined(); + }); +}); From 7c252b15d8b9511610cfe4a7faf218f626156808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 11:35:15 +0200 Subject: [PATCH 14/19] cleanup --- .../EditLocationPage/EditLocationPage.tsx | 12 +- .../EditPostureCheckPage.tsx | 22 ++- .../PostureChecksPage/postureCheckMenu.ts | 9 +- web/src/shared/utils/postureWarning.ts | 29 +--- web/tests/posture-warning.test.ts | 147 +++++++++++++----- 5 files changed, 141 insertions(+), 78 deletions(-) diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index 70d6b4a03..c5d4cdba7 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -483,13 +483,13 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { }); const handlePostureSelection = (values: (string | number)[]) => { - const next = values.filter((v): v is number => typeof v === 'number'); - confirmPostureSelectionChange( - location.posture_checks ?? [], + const next = values.filter((value): value is number => typeof value === 'number'); + confirmPostureSelectionChange({ + current: location.posture_checks ?? [], next, - postureCheckOptions, - () => setLocationPosturesAsync({ postures: next }), - ); + options: postureCheckOptions, + actionPromise: () => setLocationPosturesAsync({ postures: next }), + }); }; const openPostureChecksSelection = () => { diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index 411cff1a6..7235582ed 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -135,18 +135,16 @@ const EditPostureCheckForm = ({ const handleSubmit = () => { if (rulesChanged || locationsChanged) { - const deferred = rulesChanged && defaults.locations.size > 0; - if ( - confirmLocationSelectionChange( - [...defaults.locations], - [...values.locations], - locationOptions, - () => saveMutation.mutateAsync(values), - deferred, - ) - ) { - return; - } + const modalOpened = confirmLocationSelectionChange({ + current: defaults.locations, + next: values.locations, + options: locationOptions, + actionPromise: () => saveMutation.mutateAsync(values), + // A posture assigned to no locations after this save enforces nothing + // anywhere, so the deferred-enforcement claim would be misleading. + deferredEnforcement: rulesChanged && values.locations.size > 0, + }); + if (modalOpened) return; } saveMutation.mutate(values); }; diff --git a/web/src/pages/PostureChecksPage/postureCheckMenu.ts b/web/src/pages/PostureChecksPage/postureCheckMenu.ts index e91c0e733..74f0aab1d 100644 --- a/web/src/pages/PostureChecksPage/postureCheckMenu.ts +++ b/web/src/pages/PostureChecksPage/postureCheckMenu.ts @@ -60,9 +60,12 @@ export const buildPostureCheckMenuItems = ({ selected: new Set(row.locations), onSubmit: (selected) => { const next = selected as number[]; - confirmLocationSelectionChange(row.locations, next, locationOptions, () => - assignLocations(next), - ); + confirmLocationSelectionChange({ + current: row.locations, + next, + options: locationOptions, + actionPromise: () => assignLocations(next), + }); }, }); }, diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index aa0b859d4..2eb45bde6 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -7,11 +7,14 @@ type Option = { readonly id: number; readonly label: string }; /** Escape markdown control characters so admin-supplied labels render as literal text. */ const escapeMarkdown = (s: string) => s.replace(/[\\`*_{}[\]()#+\-.!>|~]/g, '\\$&'); -type ConfirmSelectionChangeArgs = { +type SelectionChangeArgs = { current: Iterable; next: Iterable; options: readonly Option[]; actionPromise: () => Promise; +}; + +type ConfirmSelectionChangeArgs = SelectionChangeArgs & { deferredEnforcement?: boolean; bodyMessage: (args: { changes: string }) => string; }; @@ -94,17 +97,9 @@ const confirmSelectionChange = ({ * Ids are posture-check ids; the body warns about active sessions for * this location. */ -export const confirmPostureSelectionChange = ( - current: Iterable, - next: Iterable, - options: readonly Option[], - actionPromise: () => Promise, -): boolean => +export const confirmPostureSelectionChange = (args: SelectionChangeArgs): boolean => confirmSelectionChange({ - current, - next, - options, - actionPromise, + ...args, bodyMessage: m.modal_posture_assignment_warning_body_location, }); @@ -114,17 +109,9 @@ export const confirmPostureSelectionChange = ( * to the warning body. */ export const confirmLocationSelectionChange = ( - current: Iterable, - next: Iterable, - options: readonly Option[], - actionPromise: () => Promise, - deferredEnforcement?: boolean, + args: SelectionChangeArgs & { deferredEnforcement?: boolean }, ): boolean => confirmSelectionChange({ - current, - next, - options, - actionPromise, - deferredEnforcement, + ...args, bodyMessage: m.modal_posture_assignment_warning_body_postures, }); diff --git a/web/tests/posture-warning.test.ts b/web/tests/posture-warning.test.ts index 7727ef09e..d0cec240b 100644 --- a/web/tests/posture-warning.test.ts +++ b/web/tests/posture-warning.test.ts @@ -37,32 +37,44 @@ const locOptions: Option[] = [ { id: 3, label: 'Zurich' }, ]; +const noop = async () => {}; + afterEach(() => { vi.clearAllMocks(); }); describe('confirmPostureSelectionChange', () => { it('returns false and opens nothing when the id sets are identical', () => { - const result = confirmPostureSelectionChange( - [1, 2], - [1, 2], - locOptions, - async () => {}, - ); + const result = confirmPostureSelectionChange({ + current: [1, 2], + next: [1, 2], + options: locOptions, + actionPromise: noop, + }); expect(result).toBe(false); expect(openModal).not.toHaveBeenCalled(); }); it('returns false and opens nothing when both sets are empty', () => { - const result = confirmPostureSelectionChange([], [], locOptions, async () => {}); + const result = confirmPostureSelectionChange({ + current: [], + next: [], + options: locOptions, + actionPromise: noop, + }); expect(result).toBe(false); expect(openModal).not.toHaveBeenCalled(); }); it('opens a modal with the Added group when items were added', () => { - confirmPostureSelectionChange([1], [1, 2, 3], locOptions, async () => {}); + confirmPostureSelectionChange({ + current: [1], + next: [1, 2, 3], + options: locOptions, + actionPromise: noop, + }); expect(openModal).toHaveBeenCalledOnce(); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; @@ -76,7 +88,12 @@ describe('confirmPostureSelectionChange', () => { }); it('opens a modal with the Removed group when items were removed', () => { - confirmPostureSelectionChange([1, 2, 3], [1], locOptions, async () => {}); + confirmPostureSelectionChange({ + current: [1, 2, 3], + next: [1], + options: locOptions, + actionPromise: noop, + }); expect(openModal).toHaveBeenCalledOnce(); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; @@ -88,7 +105,12 @@ describe('confirmPostureSelectionChange', () => { }); it('opens a modal with both Added and Removed groups when items changed', () => { - confirmPostureSelectionChange([1], [2, 3], locOptions, async () => {}); + confirmPostureSelectionChange({ + current: [1], + next: [2, 3], + options: locOptions, + actionPromise: noop, + }); expect(openModal).toHaveBeenCalledOnce(); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; @@ -98,35 +120,67 @@ describe('confirmPostureSelectionChange', () => { }); it('includes the location-warning body message', () => { - confirmPostureSelectionChange([1], [2], locOptions, async () => {}); + confirmPostureSelectionChange({ + current: [1], + next: [2], + options: locOptions, + actionPromise: noop, + }); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; expect(contentMd).toContain('for this location.'); }); + + it('accepts Sets as well as arrays', () => { + const result = confirmPostureSelectionChange({ + current: new Set([1, 2]), + next: new Set([2, 3]), + options: locOptions, + actionPromise: noop, + }); + + expect(result).toBe(true); + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('**Added:**'); + expect(contentMd).toContain('- Zurich'); + expect(contentMd).toContain('**Removed:**'); + expect(contentMd).toContain('- Berlin'); + }); }); describe('confirmLocationSelectionChange', () => { it('returns false when no diff and no deferredEnforcement', () => { - const result = confirmLocationSelectionChange( - [1, 2], - [1, 2], - locOptions, - async () => {}, - ); + const result = confirmLocationSelectionChange({ + current: [1, 2], + next: [1, 2], + options: locOptions, + actionPromise: noop, + }); expect(result).toBe(false); expect(openModal).not.toHaveBeenCalled(); }); it('includes the postures-warning body message', () => { - confirmLocationSelectionChange([1], [2], locOptions, async () => {}); + confirmLocationSelectionChange({ + current: [1], + next: [2], + options: locOptions, + actionPromise: noop, + }); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; expect(contentMd).toContain('for the affected locations.'); }); it('opens rules-only body when deferredEnforcement is true and no location diff', () => { - confirmLocationSelectionChange([1, 2], [1, 2], locOptions, async () => {}, true); + confirmLocationSelectionChange({ + current: [1, 2], + next: [1, 2], + options: locOptions, + actionPromise: noop, + deferredEnforcement: true, + }); expect(openModal).toHaveBeenCalledOnce(); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; @@ -136,7 +190,13 @@ describe('confirmLocationSelectionChange', () => { }); it('appends rules paragraph after locations diff when deferredEnforcement is true and diff exists', () => { - confirmLocationSelectionChange([1], [2], locOptions, async () => {}, true); + confirmLocationSelectionChange({ + current: [1], + next: [2], + options: locOptions, + actionPromise: noop, + deferredEnforcement: true, + }); expect(openModal).toHaveBeenCalledOnce(); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; @@ -149,13 +209,13 @@ describe('confirmLocationSelectionChange', () => { }); it('returns false when deferredEnforcement is false (the falsy default)', () => { - const result = confirmLocationSelectionChange( - [1], - [1], - locOptions, - async () => {}, - false, - ); + const result = confirmLocationSelectionChange({ + current: [1], + next: [1], + options: locOptions, + actionPromise: noop, + deferredEnforcement: false, + }); expect(result).toBe(false); }); @@ -170,7 +230,12 @@ describe('markdown escaping', () => { { id: 4, label: 'back`tick' }, ]; - confirmPostureSelectionChange([1], [2, 3, 4], spikyOptions, async () => {}); + confirmPostureSelectionChange({ + current: [1], + next: [2, 3, 4], + options: spikyOptions, + actionPromise: noop, + }); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; // Asterisks escaped @@ -190,7 +255,12 @@ describe('markdown escaping', () => { { id: 2, label: 'os_version_check' }, ]; - confirmPostureSelectionChange([1], [2], underscoreOptions, async () => {}); + confirmPostureSelectionChange({ + current: [1], + next: [2], + options: underscoreOptions, + actionPromise: noop, + }); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; expect(contentMd).not.toMatch(/(? { describe('unknown id fallback', () => { it('uses String(id) when an id has no matching option', () => { - confirmPostureSelectionChange( - [], - [1, 99], - [{ id: 1, label: 'Known' }], - async () => {}, - ); + confirmPostureSelectionChange({ + current: [], + next: [1, 99], + options: [{ id: 1, label: 'Known' }], + actionPromise: noop, + }); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; expect(contentMd).toContain('- 99'); @@ -215,7 +285,12 @@ describe('unknown id fallback', () => { describe('modal structure', () => { it('opens ConfirmAction with the shared title, critical variant, and actionPromise', () => { const actionPromise = async () => 'saved'; - confirmPostureSelectionChange([1], [2], locOptions, actionPromise); + confirmPostureSelectionChange({ + current: [1], + next: [2], + options: locOptions, + actionPromise, + }); const modalData = vi.mocked(openModal).mock.calls[0][1]; expect(openModal).toHaveBeenCalledWith( From a35ec850f905f0f4d06000b33d21ddcac4d5cb2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 11:43:24 +0200 Subject: [PATCH 15/19] update markdown escape --- web/src/shared/utils/postureWarning.ts | 13 +++++++-- web/tests/posture-warning.test.ts | 38 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index 2eb45bde6..f970bc45a 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -4,8 +4,17 @@ import { ModalName } from '../hooks/modalControls/modalTypes'; type Option = { readonly id: number; readonly label: string }; -/** Escape markdown control characters so admin-supplied labels render as literal text. */ -const escapeMarkdown = (s: string) => s.replace(/[\\`*_{}[\]()#+\-.!>|~]/g, '\\$&'); +/** + * Every ASCII punctuation character, all of which CommonMark guarantees are + * backslash-escapable. Escaping the whole set rather than an enumerated subset + * leaves no character to audit: `<` and `&` matter in particular, because + * `RenderMarkdown` runs `rehypeRaw`, so an unescaped tag or entity in a name + * would be parsed as HTML rather than shown to the admin as written. + */ +const MARKDOWN_PUNCTUATION = /[!"#$%&'()*+,\-./:;<=>?@[\\\]^_`{|}~]/g; + +/** Escape markdown and HTML syntax so admin-supplied labels render as literal text. */ +const escapeMarkdown = (value: string) => value.replace(MARKDOWN_PUNCTUATION, '\\$&'); type SelectionChangeArgs = { current: Iterable; diff --git a/web/tests/posture-warning.test.ts b/web/tests/posture-warning.test.ts index d0cec240b..f28895d44 100644 --- a/web/tests/posture-warning.test.ts +++ b/web/tests/posture-warning.test.ts @@ -249,6 +249,44 @@ describe('markdown escaping', () => { expect(contentMd).toContain('- plain'); }); + it('escapes HTML syntax so rehypeRaw cannot parse a tag out of a label', () => { + const htmlOptions: Option[] = [ + { id: 1, label: 'old' }, + { id: 2, label: 'bold' }, + { id: 3, label: '' }, + { id: 4, label: '&' }, + ]; + + confirmPostureSelectionChange({ + current: [1], + next: [2, 3, 4], + options: htmlOptions, + actionPromise: noop, + }); + + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).toContain('\\bold\\<\\/b\\>'); + expect(contentMd).toContain('\\'); + expect(contentMd).toContain('\\&\\;'); + }); + + it('leaves no unescaped angle bracket or ampersand in the body', () => { + const hostileOptions: Option[] = [ + { id: 1, label: 'click' }, + { id: 2, label: ' & co' }, + ]; + + confirmPostureSelectionChange({ + current: [1], + next: [2], + options: hostileOptions, + actionPromise: noop, + }); + + const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; + expect(contentMd).not.toMatch(/(?&]/); + }); + it('prevents underscores from being interpreted as italics', () => { const underscoreOptions: Option[] = [ { id: 1, label: 'old' }, From e7e73ef6dbe584149a8556eff49cbf3c54f6d24a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 11:51:04 +0200 Subject: [PATCH 16/19] remove escaping --- web/src/shared/utils/postureWarning.ts | 22 ++------ web/tests/posture-warning.test.ts | 73 ++------------------------ 2 files changed, 10 insertions(+), 85 deletions(-) diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index f970bc45a..73f5289f1 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -4,18 +4,6 @@ import { ModalName } from '../hooks/modalControls/modalTypes'; type Option = { readonly id: number; readonly label: string }; -/** - * Every ASCII punctuation character, all of which CommonMark guarantees are - * backslash-escapable. Escaping the whole set rather than an enumerated subset - * leaves no character to audit: `<` and `&` matter in particular, because - * `RenderMarkdown` runs `rehypeRaw`, so an unescaped tag or entity in a name - * would be parsed as HTML rather than shown to the admin as written. - */ -const MARKDOWN_PUNCTUATION = /[!"#$%&'()*+,\-./:;<=>?@[\\\]^_`{|}~]/g; - -/** Escape markdown and HTML syntax so admin-supplied labels render as literal text. */ -const escapeMarkdown = (value: string) => value.replace(MARKDOWN_PUNCTUATION, '\\$&'); - type SelectionChangeArgs = { current: Iterable; next: Iterable; @@ -29,9 +17,9 @@ type ConfirmSelectionChangeArgs = SelectionChangeArgs & { }; /** - * Diffs id sets, resolves labels, escapes markdown, composes the four - * states (diff only, deferred-enforcement only, both, neither) and opens - * the ConfirmAction modal. Returns `true` when a modal was opened. + * Diffs id sets, resolves labels, composes the four states (diff only, + * deferred-enforcement only, both, neither) and opens the ConfirmAction + * modal. Returns `true` when a modal was opened. */ const confirmSelectionChange = ({ current, @@ -50,10 +38,10 @@ const confirmSelectionChange = ({ const labelMap = new Map(options.map((o) => [o.id, o.label])); const sortedAdded = addedIds - .map((id) => escapeMarkdown(labelMap.get(id) ?? String(id))) + .map((id) => labelMap.get(id) ?? String(id)) .sort((a, b) => a.localeCompare(b)); const sortedRemoved = removedIds - .map((id) => escapeMarkdown(labelMap.get(id) ?? String(id))) + .map((id) => labelMap.get(id) ?? String(id)) .sort((a, b) => a.localeCompare(b)); const parts: string[] = []; diff --git a/web/tests/posture-warning.test.ts b/web/tests/posture-warning.test.ts index f28895d44..369322af7 100644 --- a/web/tests/posture-warning.test.ts +++ b/web/tests/posture-warning.test.ts @@ -221,87 +221,24 @@ describe('confirmLocationSelectionChange', () => { }); }); -describe('markdown escaping', () => { - it('escapes markdown control characters in labels', () => { +describe('labels are passed through verbatim', () => { + it('does not alter labels containing markdown syntax', () => { const spikyOptions: Option[] = [ { id: 1, label: 'plain' }, { id: 2, label: '*bold*' }, { id: 3, label: '[link](url)' }, - { id: 4, label: 'back`tick' }, ]; confirmPostureSelectionChange({ current: [1], - next: [2, 3, 4], + next: [2, 3], options: spikyOptions, actionPromise: noop, }); const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; - // Asterisks escaped - expect(contentMd).toContain('\\*bold\\*'); - expect(contentMd).not.toMatch(/(? { - const htmlOptions: Option[] = [ - { id: 1, label: 'old' }, - { id: 2, label: 'bold' }, - { id: 3, label: '' }, - { id: 4, label: '&' }, - ]; - - confirmPostureSelectionChange({ - current: [1], - next: [2, 3, 4], - options: htmlOptions, - actionPromise: noop, - }); - - const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; - expect(contentMd).toContain('\\bold\\<\\/b\\>'); - expect(contentMd).toContain('\\'); - expect(contentMd).toContain('\\&\\;'); - }); - - it('leaves no unescaped angle bracket or ampersand in the body', () => { - const hostileOptions: Option[] = [ - { id: 1, label: 'click' }, - { id: 2, label: ' & co' }, - ]; - - confirmPostureSelectionChange({ - current: [1], - next: [2], - options: hostileOptions, - actionPromise: noop, - }); - - const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; - expect(contentMd).not.toMatch(/(?&]/); - }); - - it('prevents underscores from being interpreted as italics', () => { - const underscoreOptions: Option[] = [ - { id: 1, label: 'old' }, - { id: 2, label: 'os_version_check' }, - ]; - - confirmPostureSelectionChange({ - current: [1], - next: [2], - options: underscoreOptions, - actionPromise: noop, - }); - - const contentMd = vi.mocked(openModal).mock.calls[0][1].contentMd; - expect(contentMd).not.toMatch(/(? Date: Thu, 6 Aug 2026 12:01:00 +0200 Subject: [PATCH 17/19] de-duplicate OS sort --- web/src/pages/EditPostureCheckPage/form.ts | 55 ++++++++++++++-------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/web/src/pages/EditPostureCheckPage/form.ts b/web/src/pages/EditPostureCheckPage/form.ts index 800c03c76..03159d635 100644 --- a/web/src/pages/EditPostureCheckPage/form.ts +++ b/web/src/pages/EditPostureCheckPage/form.ts @@ -135,32 +135,47 @@ export const getInitialEditPostureCheckFormValues = ( }; }; +const sortOperatingSystemState = ( + operatingSystemState: EditPostureCheckFormValues['operatingSystemState'], +) => + Object.fromEntries( + Object.entries(operatingSystemState).map(([os, state]) => [ + os, + { ...state, conditions: [...state.conditions].sort() }, + ]), + ); + export const normalizeEditPostureCheckFormValues = ( values: EditPostureCheckFormValues, ) => ({ ...values, configuredOperatingSystems: [...values.configuredOperatingSystems].sort(), locations: Array.from(values.locations).sort((left, right) => left - right), - operatingSystemState: Object.fromEntries( - Object.entries(values.operatingSystemState).map(([os, state]) => [ - os, - { ...state, conditions: [...state.conditions].sort() }, - ]), - ), + operatingSystemState: sortOperatingSystemState(values.operatingSystemState), }); -/** Projection of enforcement-related fields for `rulesChanged` comparison. */ +/** + * Projection of enforcement-related fields for `rulesChanged` comparison: + * everything except `name`, `description` and `locations`. Derived from + * `normalizeEditPostureCheckFormValues` so both comparisons share one sort + * policy. + */ export const normalizeEditPostureCheckEnforcementFields = ( - v: EditPostureCheckFormValues, -) => ({ - allowPrereleaseClient: v.allowPrereleaseClient, - configuredOperatingSystems: [...v.configuredOperatingSystems].sort(), - minimumDesktopClientVersion: v.minimumDesktopClientVersion, - minimumMobileClientVersion: v.minimumMobileClientVersion, - operatingSystemState: Object.fromEntries( - Object.entries(v.operatingSystemState).map(([os, state]) => [ - os, - { ...state, conditions: [...state.conditions].sort() }, - ]), - ), -}); + values: EditPostureCheckFormValues, +) => { + const { + allowPrereleaseClient, + configuredOperatingSystems, + minimumDesktopClientVersion, + minimumMobileClientVersion, + operatingSystemState, + } = normalizeEditPostureCheckFormValues(values); + + return { + allowPrereleaseClient, + configuredOperatingSystems, + minimumDesktopClientVersion, + minimumMobileClientVersion, + operatingSystemState, + }; +}; From 9885249c03b760de0e581b07a2299885e98ab0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 12:04:50 +0200 Subject: [PATCH 18/19] remove duplicates --- .../EditPostureCheckPage.tsx | 35 ++++++------- web/src/shared/utils/postureWarning.ts | 51 ++++++++++--------- 2 files changed, 41 insertions(+), 45 deletions(-) diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index 7235582ed..34ca67b0d 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -125,27 +125,22 @@ const EditPostureCheckForm = ({ [defaults, values], ); - const locationsChanged = useMemo(() => { - if (values.locations.size !== defaults.locations.size) return true; - for (const id of values.locations) { - if (!defaults.locations.has(id)) return true; - } - return false; - }, [values.locations, defaults.locations]); - const handleSubmit = () => { - if (rulesChanged || locationsChanged) { - const modalOpened = confirmLocationSelectionChange({ - current: defaults.locations, - next: values.locations, - options: locationOptions, - actionPromise: () => saveMutation.mutateAsync(values), - // A posture assigned to no locations after this save enforces nothing - // anywhere, so the deferred-enforcement claim would be misleading. - deferredEnforcement: rulesChanged && values.locations.size > 0, - }); - if (modalOpened) return; - } + // The helper diffs the location sets itself and returns false when there is + // nothing to confirm, which covers the submit that changed only the name or + // description. + const modalOpened = confirmLocationSelectionChange({ + current: defaults.locations, + next: values.locations, + options: locationOptions, + actionPromise: () => saveMutation.mutateAsync(values), + // A posture assigned to no locations after this save enforces nothing + // anywhere, so the deferred-enforcement claim would be misleading. + deferredEnforcement: rulesChanged && values.locations.size > 0, + }); + + if (modalOpened) return; + saveMutation.mutate(values); }; diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index 73f5289f1..2cbeb05e0 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -16,6 +16,26 @@ type ConfirmSelectionChangeArgs = SelectionChangeArgs & { bodyMessage: (args: { changes: string }) => string; }; +/** + * One bold heading followed by the ids as label bullets, sorted by label, or + * `null` when there are no ids so the group is omitted along with its heading. + */ +const formatGroup = ( + heading: string, + ids: number[], + labelFor: (id: number) => string, +) => { + if (ids.length === 0) return null; + + const bullets = ids + .map(labelFor) + .sort((left, right) => left.localeCompare(right)) + .map((label) => `- ${label}`) + .join('\n'); + + return `**${heading}**\n\n${bullets}`; +}; + /** * Diffs id sets, resolves labels, composes the four states (diff only, * deferred-enforcement only, both, neither) and opens the ConfirmAction @@ -35,32 +55,13 @@ const confirmSelectionChange = ({ const addedIds = [...nextSet].filter((id) => !currentSet.has(id)); const removedIds = [...currentSet].filter((id) => !nextSet.has(id)); - const labelMap = new Map(options.map((o) => [o.id, o.label])); - - const sortedAdded = addedIds - .map((id) => labelMap.get(id) ?? String(id)) - .sort((a, b) => a.localeCompare(b)); - const sortedRemoved = removedIds - .map((id) => labelMap.get(id) ?? String(id)) - .sort((a, b) => a.localeCompare(b)); + const labelMap = new Map(options.map((option) => [option.id, option.label])); + const labelFor = (id: number) => labelMap.get(id) ?? String(id); - const parts: string[] = []; - - if (sortedAdded.length > 0) { - parts.push( - `**${m.modal_posture_assignment_warning_added()}**\n\n${sortedAdded - .map((name) => `- ${name}`) - .join('\n')}`, - ); - } - - if (sortedRemoved.length > 0) { - parts.push( - `**${m.modal_posture_assignment_warning_removed()}**\n\n${sortedRemoved - .map((name) => `- ${name}`) - .join('\n')}`, - ); - } + const parts = [ + formatGroup(m.modal_posture_assignment_warning_added(), addedIds, labelFor), + formatGroup(m.modal_posture_assignment_warning_removed(), removedIds, labelFor), + ].filter((part): part is string => part !== null); const changes = parts.length > 0 ? parts.join('\n\n') : null; const hasDiff = changes !== null; From 0ccd595f9f1f67724ea2d5d87bbfd2d0462abc0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Thu, 6 Aug 2026 12:17:42 +0200 Subject: [PATCH 19/19] cleanup --- .../EditLocationPage/EditLocationPage.tsx | 18 ++------- .../EditPostureCheckPage.tsx | 4 +- .../PostureChecksPage/postureCheckMenu.ts | 4 +- web/src/shared/utils/postureWarning.ts | 24 +++++++---- web/tests/posture-warning.test.ts | 40 +++++++++---------- 5 files changed, 43 insertions(+), 47 deletions(-) diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index c5d4cdba7..aac4b84ca 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -43,7 +43,7 @@ import { canUseEnterpriseFeature, } from '../../shared/utils/license'; import { smallestNetworkCapacity } from '../../shared/utils/network'; -import { confirmPostureSelectionChange } from '../../shared/utils/postureWarning'; +import { confirmLocationPostureChange } from '../../shared/utils/postureWarning'; import { Validate } from '../../shared/validate'; import postureCheckShield from './assets/posture_check_shield.png'; import { getPostureChecksSectionState } from './postureChecksSection'; @@ -392,14 +392,6 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { [postureChecks], ); - const assignedPostureChecks = useMemo(() => { - const labelByOption = new Map(postureCheckOptions.map((o) => [o.id, o.label])); - return location.posture_checks?.map((id) => ({ - id, - label: labelByOption.get(id) ?? String(id), - })); - }, [location.posture_checks, postureCheckOptions]); - const serviceLocationLabelContent = useMemo(() => { if (!serviceLocationLocked) return undefined; return ( @@ -484,7 +476,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { const handlePostureSelection = (values: (string | number)[]) => { const next = values.filter((value): value is number => typeof value === 'number'); - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: location.posture_checks ?? [], next, options: postureCheckOptions, @@ -987,11 +979,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => {
postureCheck.id), - ) - } + selected={new Set(location.posture_checks)} modalTitle={m.location_posture_checks_select()} editText={m.location_posture_checks_edit()} editIcon={IconKind.Edit} diff --git a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx index 34ca67b0d..028b4a6a6 100644 --- a/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx +++ b/web/src/pages/EditPostureCheckPage/EditPostureCheckPage.tsx @@ -35,7 +35,7 @@ import { getDevicePostureVersionMetadataQueryOptions, getLocationsQueryOptions, } from '../../shared/query'; -import { confirmLocationSelectionChange } from '../../shared/utils/postureWarning'; +import { confirmPostureLocationChange } from '../../shared/utils/postureWarning'; import { buildAddPostureCheckRequest } from '../AddPostureCheckWizardPage/payload'; import { getDeletePostureCheckModalData } from '../PostureChecksPage/postureChecks'; import { @@ -129,7 +129,7 @@ const EditPostureCheckForm = ({ // The helper diffs the location sets itself and returns false when there is // nothing to confirm, which covers the submit that changed only the name or // description. - const modalOpened = confirmLocationSelectionChange({ + const modalOpened = confirmPostureLocationChange({ current: defaults.locations, next: values.locations, options: locationOptions, diff --git a/web/src/pages/PostureChecksPage/postureCheckMenu.ts b/web/src/pages/PostureChecksPage/postureCheckMenu.ts index 74f0aab1d..b32102964 100644 --- a/web/src/pages/PostureChecksPage/postureCheckMenu.ts +++ b/web/src/pages/PostureChecksPage/postureCheckMenu.ts @@ -7,7 +7,7 @@ import type { MenuItemsGroup } from '../../shared/defguard-ui/components/Menu/ty import { Snackbar } from '../../shared/defguard-ui/providers/snackbar/snackbar'; import { openModal } from '../../shared/hooks/modalControls/modalsSubjects'; import { ModalName } from '../../shared/hooks/modalControls/modalTypes'; -import { confirmLocationSelectionChange } from '../../shared/utils/postureWarning'; +import { confirmPostureLocationChange } from '../../shared/utils/postureWarning'; import { getDeletePostureCheckModalData, type PostureCheckRow } from './postureChecks'; type LocationOption = SelectionOption; @@ -60,7 +60,7 @@ export const buildPostureCheckMenuItems = ({ selected: new Set(row.locations), onSubmit: (selected) => { const next = selected as number[]; - confirmLocationSelectionChange({ + confirmPostureLocationChange({ current: row.locations, next, options: locationOptions, diff --git a/web/src/shared/utils/postureWarning.ts b/web/src/shared/utils/postureWarning.ts index 2cbeb05e0..c82f6178b 100644 --- a/web/src/shared/utils/postureWarning.ts +++ b/web/src/shared/utils/postureWarning.ts @@ -91,22 +91,30 @@ const confirmSelectionChange = ({ }; /** - * Warn when the set of assigned posture checks on a location changes. - * Ids are posture-check ids; the body warns about active sessions for - * this location. + * Warn before changing which posture checks apply to a location. `current` and + * `next` hold posture-check ids; the body warns about active sessions on this + * location. + * + * Returns `true` when a modal was opened, which is not the same as the admin + * agreeing: the modal is fired and forgotten, and `actionPromise` runs only if + * they confirm. A `false` return means there was nothing to warn about, so the + * caller should proceed with its own save. */ -export const confirmPostureSelectionChange = (args: SelectionChangeArgs): boolean => +export const confirmLocationPostureChange = (args: SelectionChangeArgs): boolean => confirmSelectionChange({ ...args, bodyMessage: m.modal_posture_assignment_warning_body_location, }); /** - * Warn when the set of assigned locations on a posture check changes. - * When `deferredEnforcement` is true, appends the rules-deferred paragraph - * to the warning body. + * Warn before changing which locations a posture check applies to. `current` and + * `next` hold location ids. When `deferredEnforcement` is true, appends the + * rules-deferred paragraph to the warning body. + * + * Returns `true` when a modal was opened; see + * {@link confirmLocationPostureChange} for what that does and does not mean. */ -export const confirmLocationSelectionChange = ( +export const confirmPostureLocationChange = ( args: SelectionChangeArgs & { deferredEnforcement?: boolean }, ): boolean => confirmSelectionChange({ diff --git a/web/tests/posture-warning.test.ts b/web/tests/posture-warning.test.ts index 369322af7..f258ccc2b 100644 --- a/web/tests/posture-warning.test.ts +++ b/web/tests/posture-warning.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { openModal } from '../src/shared/hooks/modalControls/modalsSubjects'; vi.mock('../src/shared/hooks/modalControls/modalsSubjects', () => ({ @@ -25,8 +25,8 @@ vi.mock('../src/paraglide/messages', () => ({ })); import { - confirmLocationSelectionChange, - confirmPostureSelectionChange, + confirmLocationPostureChange, + confirmPostureLocationChange, } from '../src/shared/utils/postureWarning'; type Option = { readonly id: number; readonly label: string }; @@ -43,9 +43,9 @@ afterEach(() => { vi.clearAllMocks(); }); -describe('confirmPostureSelectionChange', () => { +describe('confirmLocationPostureChange', () => { it('returns false and opens nothing when the id sets are identical', () => { - const result = confirmPostureSelectionChange({ + const result = confirmLocationPostureChange({ current: [1, 2], next: [1, 2], options: locOptions, @@ -57,7 +57,7 @@ describe('confirmPostureSelectionChange', () => { }); it('returns false and opens nothing when both sets are empty', () => { - const result = confirmPostureSelectionChange({ + const result = confirmLocationPostureChange({ current: [], next: [], options: locOptions, @@ -69,7 +69,7 @@ describe('confirmPostureSelectionChange', () => { }); it('opens a modal with the Added group when items were added', () => { - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: [1], next: [1, 2, 3], options: locOptions, @@ -88,7 +88,7 @@ describe('confirmPostureSelectionChange', () => { }); it('opens a modal with the Removed group when items were removed', () => { - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: [1, 2, 3], next: [1], options: locOptions, @@ -105,7 +105,7 @@ describe('confirmPostureSelectionChange', () => { }); it('opens a modal with both Added and Removed groups when items changed', () => { - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: [1], next: [2, 3], options: locOptions, @@ -120,7 +120,7 @@ describe('confirmPostureSelectionChange', () => { }); it('includes the location-warning body message', () => { - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: [1], next: [2], options: locOptions, @@ -132,7 +132,7 @@ describe('confirmPostureSelectionChange', () => { }); it('accepts Sets as well as arrays', () => { - const result = confirmPostureSelectionChange({ + const result = confirmLocationPostureChange({ current: new Set([1, 2]), next: new Set([2, 3]), options: locOptions, @@ -148,9 +148,9 @@ describe('confirmPostureSelectionChange', () => { }); }); -describe('confirmLocationSelectionChange', () => { +describe('confirmPostureLocationChange', () => { it('returns false when no diff and no deferredEnforcement', () => { - const result = confirmLocationSelectionChange({ + const result = confirmPostureLocationChange({ current: [1, 2], next: [1, 2], options: locOptions, @@ -162,7 +162,7 @@ describe('confirmLocationSelectionChange', () => { }); it('includes the postures-warning body message', () => { - confirmLocationSelectionChange({ + confirmPostureLocationChange({ current: [1], next: [2], options: locOptions, @@ -174,7 +174,7 @@ describe('confirmLocationSelectionChange', () => { }); it('opens rules-only body when deferredEnforcement is true and no location diff', () => { - confirmLocationSelectionChange({ + confirmPostureLocationChange({ current: [1, 2], next: [1, 2], options: locOptions, @@ -190,7 +190,7 @@ describe('confirmLocationSelectionChange', () => { }); it('appends rules paragraph after locations diff when deferredEnforcement is true and diff exists', () => { - confirmLocationSelectionChange({ + confirmPostureLocationChange({ current: [1], next: [2], options: locOptions, @@ -209,7 +209,7 @@ describe('confirmLocationSelectionChange', () => { }); it('returns false when deferredEnforcement is false (the falsy default)', () => { - const result = confirmLocationSelectionChange({ + const result = confirmPostureLocationChange({ current: [1], next: [1], options: locOptions, @@ -229,7 +229,7 @@ describe('labels are passed through verbatim', () => { { id: 3, label: '[link](url)' }, ]; - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: [1], next: [2, 3], options: spikyOptions, @@ -244,7 +244,7 @@ describe('labels are passed through verbatim', () => { describe('unknown id fallback', () => { it('uses String(id) when an id has no matching option', () => { - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: [], next: [1, 99], options: [{ id: 1, label: 'Known' }], @@ -260,7 +260,7 @@ describe('unknown id fallback', () => { describe('modal structure', () => { it('opens ConfirmAction with the shared title, critical variant, and actionPromise', () => { const actionPromise = async () => 'saved'; - confirmPostureSelectionChange({ + confirmLocationPostureChange({ current: [1], next: [2], options: locOptions,