Alerting: Fix inheritance of the timing options for policy tree (#99398)

public/app/features/alerting/unified/NotificationPoliciesPage.test.tsx

@@ -296,14 +296,14 @@ describe.each([
   });
 
   it('allows user to reload and update policies if its been changed by another user', async () => {
-    jest.retryTimes(2);
     const { user } = renderNotificationPolicies();
+    const NEW_INTERVAL = '12h';
 
     await getRootRoute();
 
     const existingConfig = getAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME);
     const modifiedConfig = produce(existingConfig, (draft) => {
-      draft.alertmanager_config.route!.group_interval = '12h';
+      draft.alertmanager_config.route!.group_interval = NEW_INTERVAL;
     });
     setAlertmanagerConfig(GRAFANA_RULES_SOURCE_NAME, modifiedConfig);
 
@@ -317,11 +317,8 @@ describe.each([
     await user.click(screen.getByRole('button', { name: /cancel/i }));
     await user.click(screen.getByRole('button', { name: /reload policies/i }));
 
-    await openDefaultPolicyEditModal();
-    await user.click(await screen.findByRole('button', { name: /update default policy/i }));
-    expect(await screen.findByText(/updated notification policies/i)).toBeInTheDocument();
-    // TODO: Check if test flakiness/length can be improved
-  }, 60000);
+    expect((await screen.findAllByTestId('timing-options'))[0]).toHaveTextContent(NEW_INTERVAL);
+  });
 
   it('Should be able to delete an empty route', async () => {
     const defaultConfig: AlertManagerCortexConfig = {

public/app/features/alerting/unified/components/notification-policies/NotificationPoliciesList.tsx

@@ -1,3 +1,4 @@
+import { defaults } from 'lodash';
 import { useEffect, useMemo, useState } from 'react';
 import { useAsyncFn } from 'react-use';
 
@@ -23,6 +24,7 @@ import { InsertPosition } from './../../utils/routeTree';
 import { NotificationPoliciesFilter, findRoutesByMatchers, findRoutesMatchingPredicate } from './Filters';
 import { useAddPolicyModal, useAlertGroupsModal, useDeletePolicyModal, useEditPolicyModal } from './Modals';
 import { Policy } from './Policy';
+import { TIMING_OPTIONS_DEFAULTS } from './timingOptions';
 import {
   useAddNotificationPolicy,
   useDeleteNotificationPolicy,
@@ -218,34 +220,31 @@ export const NotificationPoliciesList = () => {
       )}
       {hasPoliciesData && (
         <Stack direction="column" gap={1}>
-          {rootRoute && (
-            <NotificationPoliciesFilter
-              onChangeMatchers={setLabelMatchersFilter}
-              onChangeReceiver={setContactPointFilter}
-              matchingCount={routesMatchingFilters.matchedRoutesWithPath.size}
-            />
-          )}
-          {rootRoute && (
-            <Policy
-              receivers={receivers}
-              currentRoute={rootRoute}
-              contactPointsState={contactPointsState.receivers}
-              readOnly={!hasConfigurationAPI}
-              provisioned={rootRoute[ROUTES_META_SYMBOL]?.provisioned}
-              alertManagerSourceName={selectedAlertmanager}
-              onAddPolicy={openAddModal}
-              onEditPolicy={openEditModal}
-              onDeletePolicy={openDeleteModal}
-              onShowAlertInstances={showAlertGroupsModal}
-              routesMatchingFilters={routesMatchingFilters}
-              matchingInstancesPreview={{
-                groupsMap: routeAlertGroupsMap,
-                enabled: Boolean(canSeeAlertGroups && !instancesPreviewError),
-              }}
-              isAutoGenerated={false}
-              isDefaultPolicy
-            />
-          )}
+          <NotificationPoliciesFilter
+            onChangeMatchers={setLabelMatchersFilter}
+            onChangeReceiver={setContactPointFilter}
+            matchingCount={routesMatchingFilters.matchedRoutesWithPath.size}
+          />
+          <Policy
+            receivers={receivers}
+            // add the timing defaults to the default policy to make sure child policies inherit properly
+            currentRoute={defaults(rootRoute, TIMING_OPTIONS_DEFAULTS)}
+            contactPointsState={contactPointsState.receivers}
+            readOnly={!hasConfigurationAPI}
+            provisioned={rootRoute[ROUTES_META_SYMBOL]?.provisioned}
+            alertManagerSourceName={selectedAlertmanager}
+            onAddPolicy={openAddModal}
+            onEditPolicy={openEditModal}
+            onDeletePolicy={openDeleteModal}
+            onShowAlertInstances={showAlertGroupsModal}
+            routesMatchingFilters={routesMatchingFilters}
+            matchingInstancesPreview={{
+              groupsMap: routeAlertGroupsMap,
+              enabled: Boolean(canSeeAlertGroups && !instancesPreviewError),
+            }}
+            isAutoGenerated={false}
+            isDefaultPolicy
+          />
         </Stack>
       )}
       {addModal}

public/app/features/alerting/unified/components/notification-policies/Policy.test.tsx

@@ -136,7 +136,7 @@ describe('Policy', () => {
     expect(within(firstPolicy).getByTestId('contact-point')).toHaveTextContent('provisioned-contact-point');
     expect(within(firstPolicy).getByTestId('mute-timings')).toHaveTextContent('Muted when mt-1');
     expect(within(firstPolicy).getByTestId('active-timings')).toHaveTextContent('Active when mt-2');
-    expect(within(firstPolicy).getByTestId('inherited-properties')).toHaveTextContent('Inherited2 properties');
+    expect(within(firstPolicy).getByTestId('inherited-properties')).toHaveTextContent('Inherited4 properties');
 
     // second custom policy should be correct
     const secondPolicy = customPolicies[1];
@@ -144,7 +144,7 @@ describe('Policy', () => {
     expect(within(secondPolicy).queryByTestId('continue-matching')).not.toBeInTheDocument();
     expect(within(secondPolicy).queryByTestId('mute-timings')).not.toBeInTheDocument();
     expect(within(secondPolicy).queryByTestId('active-timings')).not.toBeInTheDocument();
-    expect(within(secondPolicy).getByTestId('inherited-properties')).toHaveTextContent('Inherited3 properties');
+    expect(within(secondPolicy).getByTestId('inherited-properties')).toHaveTextContent('Inherited5 properties');
 
     // third custom policy should be correct
     const thirdPolicy = customPolicies[2];
@@ -381,8 +381,8 @@ const mockRoutes: RouteWithID = {
     },
   ],
   group_wait: '30s',
-  group_interval: undefined,
-  repeat_interval: undefined,
+  group_interval: '5m',
+  repeat_interval: '4h',
 };
 
 describe('isAutoGeneratedRootAndSimplifiedEnabled', () => {

public/app/features/alerting/unified/components/notification-policies/Policy.tsx

@@ -1,5 +1,5 @@
 import { css } from '@emotion/css';
-import { defaults, isArray, sumBy, uniqueId } from 'lodash';
+import { isArray, sumBy, uniqueId } from 'lodash';
 import pluralize from 'pluralize';
 import * as React from 'react';
 import { FC, Fragment, ReactNode, useState } from 'react';
@@ -51,7 +51,9 @@ import { GrafanaPoliciesExporter } from '../export/GrafanaPoliciesExporter';
 
 import { Matchers } from './Matchers';
 import { RoutesMatchingFilters } from './NotificationPoliciesList';
-import { TIMING_OPTIONS_DEFAULTS, TimingOptions } from './timingOptions';
+import { TimingOptions } from './timingOptions';
+
+const POLICIES_PER_PAGE = 20;
 
 interface PolicyComponentProps {
   receivers?: Receiver[];
@@ -172,8 +174,6 @@ const Policy = (props: PolicyComponentProps) => {
     errors.push(error);
   });
 
-  const POLICIES_PER_PAGE = 20;
-
   const [visibleChildPolicies, setVisibleChildPolicies] = useState(POLICIES_PER_PAGE);
 
   // build the menu actions for our policy
@@ -511,12 +511,7 @@ function MetadataRow({
             <TimeIntervals timings={activeTimings} alertManagerSourceName={alertManagerSourceName} />
           </MetaText>
         )}
-        {timingOptions && (
-          // for the default policy we will also merge the default timings, that way a user can observe what the timing options would be
-          <TimingOptionsMeta
-            timingOptions={isDefaultPolicy ? defaults(timingOptions, TIMING_OPTIONS_DEFAULTS) : timingOptions}
-          />
-        )}
+        {timingOptions && <TimingOptionsMeta timingOptions={timingOptions} />}
         {hasInheritedProperties && (
           <>
             <MetaText icon="corner-down-right-alt" data-testid="inherited-properties">