This is an automated email from the ASF dual-hosted git repository.
sandeepk318 pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new 290e9eae77 AMBARI-26657. Fix Add Service wizard Next button and Select
Version/Notifications steps (#4219)
290e9eae77 is described below
commit 290e9eae77212320956454f6a93a1f18c1fa15e4
Author: Jefferson Almeida <[email protected]>
AuthorDate: Sat Sep 12 03:08:12 2026 -0300
AMBARI-26657. Fix Add Service wizard Next button and Select
Version/Notifications steps (#4219)
* AMBARI-26657. Dedupe Select Version tabs by repository_version
The Install Wizard's Select Version step showed a duplicate tab per
stack line (e.g. two tabs pointing at the exact same install), because
/api/v1/version_definitions genuinely returns two VersionDefinition
entries per stack line once a stack has a registered repository_version:
a generic default one (stack_default: true) and a more specific alias
(stack_default: false), both carrying the identical repository_version.
Step1.tsx rendered a tab per item straight off sortedItems with no
deduplication, so both showed up.
Group definitions.items by repository_version before sorting, keeping
the stack_default entry when both are present.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
* AMBARI-26657. Stop MISC > Notifications fields from blocking the wizard
The Customize Services step's MISC > Notifications category (SMTP
Host/Port/FROM Email/TO Email/etc.) always showed "This is required"
for empty fields and blocked wizard progress, even though these fields
are meant to be optional by default.
data/configs/alert_notifications.ts already carries isRequired: false
on every field, but addAlertNotificationProperties() in Step7/index.tsx
never read that flag when building each property's propertyAttributes,
so propertyAttributes.empty_value_valid was left undefined.
ConfigUtils's validateInput() treats a missing/falsy empty_value_valid
as required, so every empty Notifications field failed validation.
Map empty_value_valid: !isRequired into each field's propertyAttributes,
scoped to the alert_notifications property list. Also lifted the
function out of the component closure into an exported, pure
addAlertNotificationProperties(wizardName, updatedConfigProperties) so
it is directly unit-testable.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
* AMBARI-26657. Fix Add Service wizard Next button silently failing past
step1
jumpToStep() rejects any forward jump (targetStep > activeStep) unless
called with isImperitiveJump=true. saveServicesAndContinue() called it
without that flag when advancing past Add Service step1, so clicking
Next after selecting services did nothing - no error, no navigation.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
* AMBARI-26657. Fix Add Service wizard Next button failing on steps past
step1
Same root cause as the step1 fix: the addService-specific forward-
navigation branches in Step5 (Assign Masters), Step6 (Assign Slaves and
Clients), and Step7 (Configurations) all called jumpToStep(nextStep)
without isImperitiveJump=true. Since canJumpFromCurrentStep() rejects
any targetStep > activeStep unless that flag is set, clicking Next
silently did nothing on every step of the Add Service wizard past
step1, not just step1.
Also fixes a Step7 test whose assertion had the bug baked in (expected
jumpToStep to be called without the true flag).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
* AMBARI-26657. Add regression tests for the forward jumpToStep fixes
Cover the shared root cause behind the Add Service wizard Next-button
fixes: useStepWizard's jumpToStep() silently no-ops on a forward jump
unless isImperitiveJump=true is passed, which is exactly what Step4/
Step5/Step6/Step7 were missing.
- useStepWizard.test.tsx (renamed from .ts to allow JSX): asserts a
forward jumpToStep() call is rejected without the flag and succeeds
with it, proving the gating mechanism the fix relies on.
- Step5.test.tsx: renders Step5 in addService mode, clicks Next, and
asserts jumpToStep is called with (nextStep, true) - a direct
regression test for this fix. Confirmed it fails if the fix is
reverted (jumpToStep(nextStep) without the flag hangs on
isNextEnabled/waitFor and the assertion never matches).
Step4 and Step6 have the identical one-line fix but their existing
test scaffolding mocks out the Table/AssignMasters children in a way
that makes reaching the Next click non-trivial without deeper test
restructuring; left as a manual QA item (see the wizard end-to-end
walkthrough in the PR description) rather than force a fragile test.
---------
Co-authored-by: Jeff Almeida <[email protected]>
---
...seStepWizard.test.ts => useStepWizard.test.tsx} | 41 ++++++-
.../latest/src/screens/ClusterWizard/Step1.tsx | 16 ++-
.../latest/src/screens/ClusterWizard/Step4.tsx | 2 +-
.../src/screens/ClusterWizard/Step5.test.tsx | 41 ++++++-
.../latest/src/screens/ClusterWizard/Step5.tsx | 2 +-
.../latest/src/screens/ClusterWizard/Step6.tsx | 2 +-
.../src/screens/ClusterWizard/Step7/index.test.tsx | 23 +++-
.../src/screens/ClusterWizard/Step7/index.tsx | 130 +++++++++++----------
.../CommonConfigs/ConfigUtils.theme.test.ts | 59 ++++++++++
9 files changed, 246 insertions(+), 70 deletions(-)
diff --git a/ambari-web/latest/src/hooks/useStepWizard.test.ts
b/ambari-web/latest/src/hooks/useStepWizard.test.tsx
similarity index 60%
rename from ambari-web/latest/src/hooks/useStepWizard.test.ts
rename to ambari-web/latest/src/hooks/useStepWizard.test.tsx
index 9a9e2719ca..5336bd44a7 100644
--- a/ambari-web/latest/src/hooks/useStepWizard.test.ts
+++ b/ambari-web/latest/src/hooks/useStepWizard.test.tsx
@@ -16,8 +16,11 @@
* limitations under the License.
*/
+import { act, renderHook } from "@testing-library/react";
+import type { PropsWithChildren } from "react";
+import { MemoryRouter } from "react-router-dom";
import { describe, expect, it } from "vitest";
-import {
+import useStepWizard, {
getAdjacentVisibleStep,
getVisibleStepNumbers,
} from "./useStepWizard";
@@ -48,3 +51,39 @@ describe("step wizard navigation", () => {
expect(getAdjacentVisibleStep(steps, 2, 1)).toBe(3);
});
});
+
+describe("jumpToStep forward-jump gating (AMBARI-26657)", () => {
+ function wrapper({ children }: PropsWithChildren) {
+ return <MemoryRouter>{children}</MemoryRouter>;
+ }
+
+ const threeSteps = () => ({
+ 0: step(),
+ 1: step(),
+ 2: step(),
+ });
+
+ it("does not advance on a forward jump called without isImperitiveJump", ()
=> {
+ const { result } = renderHook(() => useStepWizard(threeSteps(), 0), {
+ wrapper,
+ });
+
+ act(() => {
+ result.current.jumpToStep(2);
+ });
+
+ expect(result.current.activeStep).toBe(0);
+ });
+
+ it("advances on a forward jump called with isImperitiveJump=true", () => {
+ const { result } = renderHook(() => useStepWizard(threeSteps(), 0), {
+ wrapper,
+ });
+
+ act(() => {
+ result.current.jumpToStep(2, true);
+ });
+
+ expect(result.current.activeStep).toBe(2);
+ });
+});
diff --git a/ambari-web/latest/src/screens/ClusterWizard/Step1.tsx
b/ambari-web/latest/src/screens/ClusterWizard/Step1.tsx
index 1f44f1f343..1f5af954a4 100644
--- a/ambari-web/latest/src/screens/ClusterWizard/Step1.tsx
+++ b/ambari-web/latest/src/screens/ClusterWizard/Step1.tsx
@@ -193,7 +193,21 @@ export default function Step1({ wizardName =
"clusterCreation" }) {
}
const definitions: VersionDefinitionResponse =
await VersionsApi.getVersionDefinitions(stackName);
- const sortedItems = [...(definitions.items || [])].sort((a: any, b:
any) => {
+ // Ambari's /version_definitions advertises the same repository_version
+ // twice for a stack line that already has one: once as the generic
+ // "<stack>-<stack_version>" default (stack_default=true) and once as
+ // the more specific "<stack>-<stack_version>-<repository_version>"
+ // alias (stack_default=false) - both resolve to the same install.
+ // Keep one tab per repository_version, preferring the default entry.
+ const dedupedByRepoVersion = new Map<string, any>();
+ (definitions.items || []).forEach((item: any) => {
+ const key = item.VersionDefinition.repository_version ||
item.VersionDefinition.id;
+ const existing = dedupedByRepoVersion.get(key);
+ if (!existing || (item.VersionDefinition.stack_default &&
!existing.VersionDefinition.stack_default)) {
+ dedupedByRepoVersion.set(key, item);
+ }
+ });
+ const sortedItems = Array.from(dedupedByRepoVersion.values()).sort((a:
any, b: any) => {
const versionA = parseFloat(a.VersionDefinition.id.split("-")[1]);
const versionB = parseFloat(b.VersionDefinition.id.split("-")[1]);
diff --git a/ambari-web/latest/src/screens/ClusterWizard/Step4.tsx
b/ambari-web/latest/src/screens/ClusterWizard/Step4.tsx
index c7ad1820cd..7cc8717f86 100644
--- a/ambari-web/latest/src/screens/ClusterWizard/Step4.tsx
+++ b/ambari-web/latest/src/screens/ClusterWizard/Step4.tsx
@@ -152,7 +152,7 @@ export default function Step4({ wizardName =
"clusterCreation" }) {
if (wizardName === "addService") {
const nextStep = nextAddServiceStep(1, flow);
await Promise.resolve(flushStateToDb("jump", nextStep));
- jumpToStep(nextStep);
+ jumpToStep(nextStep, true);
} else {
await Promise.resolve(flushStateToDb("next"));
handleNextImperitive();
diff --git a/ambari-web/latest/src/screens/ClusterWizard/Step5.test.tsx
b/ambari-web/latest/src/screens/ClusterWizard/Step5.test.tsx
index d03eadbc7c..0821c82f44 100644
--- a/ambari-web/latest/src/screens/ClusterWizard/Step5.test.tsx
+++ b/ambari-web/latest/src/screens/ClusterWizard/Step5.test.tsx
@@ -17,8 +17,8 @@
*/
import { createContext } from "react";
-import { fireEvent, render, screen, waitFor } from "@testing-library/react";
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { cleanup, fireEvent, render, screen, waitFor } from
"@testing-library/react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { ContextWrapper } from ".";
const mocks = vi.hoisted(() => ({
@@ -51,6 +51,7 @@ describe("Assign Masters validation", () => {
vi.clearAllMocks();
mocks.flushStateToDb.mockResolvedValue(undefined);
});
+ afterEach(() => cleanup());
it("requires Continue Anyway before advancing with matching issues", async
() => {
const value = {
@@ -104,5 +105,41 @@ describe("Assign Masters validation", () => {
expect(mocks.handleNextImperitive).toHaveBeenCalledOnce();
});
});
+
+ it("jumps forward with isImperitiveJump=true when advancing the Add Service
wizard (AMBARI-26657)", async () => {
+ const jumpToStep = vi.fn();
+ const value = {
+ state: {
+ addServiceSteps: {
+ SERVICES: { data: { services: {}, addServiceFlow: {} } },
+ },
+ },
+ dispatch: vi.fn(),
+ flushStateToDb: mocks.flushStateToDb,
+ installedHosts: [],
+ installedServices: [],
+ stepWizardUtilities: {
+ currentStep: { canGoBack: true, name: "MASTERS" },
+ handleNextImperitive: mocks.handleNextImperitive,
+ handleBackImperitive: vi.fn(),
+ jumpToStep,
+ },
+ };
+ const WizardContext = createContext(value);
+
+ render(
+ <ContextWrapper.Provider value={{ Context: WizardContext }}>
+ <WizardContext.Provider value={value}>
+ <Step5 wizardName="addService" />
+ </WizardContext.Provider>
+ </ContextWrapper.Provider>,
+ );
+
+ fireEvent.click(screen.getByRole("button", { name: "NEXT" }));
+
+ await waitFor(() => {
+ expect(jumpToStep).toHaveBeenCalledWith(3, true);
+ });
+ });
});
diff --git a/ambari-web/latest/src/screens/ClusterWizard/Step5.tsx
b/ambari-web/latest/src/screens/ClusterWizard/Step5.tsx
index 53d4fb391f..9638ef0d8e 100644
--- a/ambari-web/latest/src/screens/ClusterWizard/Step5.tsx
+++ b/ambari-web/latest/src/screens/ClusterWizard/Step5.tsx
@@ -143,7 +143,7 @@ function Step5({ wizardName = "clusterCreation" }) {
if (wizardName === "addService") {
const nextStep = nextAddServiceStep(2, addServiceFlow);
await Promise.resolve(flushStateToDb("jump", nextStep));
- jumpToStep(nextStep);
+ jumpToStep(nextStep, true);
} else if (hasValidationIssues) {
setShowValidationIssuesModal(true);
} else {
diff --git a/ambari-web/latest/src/screens/ClusterWizard/Step6.tsx
b/ambari-web/latest/src/screens/ClusterWizard/Step6.tsx
index 9f235856dd..77f842cca5 100644
--- a/ambari-web/latest/src/screens/ClusterWizard/Step6.tsx
+++ b/ambari-web/latest/src/screens/ClusterWizard/Step6.tsx
@@ -446,7 +446,7 @@ function Step6({ wizardName = "clusterCreation" }:
Step6Props) {
if (wizardName === "addService") {
const nextStep = nextAddServiceStep(3, addServiceFlow);
await Promise.resolve(flushStateToDb("jump", nextStep));
- jumpToStep(nextStep);
+ jumpToStep(nextStep, true);
} else {
await Promise.resolve(flushStateToDb("next"));
handleNextImperitive();
diff --git a/ambari-web/latest/src/screens/ClusterWizard/Step7/index.test.tsx
b/ambari-web/latest/src/screens/ClusterWizard/Step7/index.test.tsx
index 81fe76cf35..3d8b3c6c52 100644
--- a/ambari-web/latest/src/screens/ClusterWizard/Step7/index.test.tsx
+++ b/ambari-web/latest/src/screens/ClusterWizard/Step7/index.test.tsx
@@ -229,6 +229,7 @@ vi.mock("../../../components/StepWizard/WizardFooter", ()
=> ({
}));
import Step7, {
+ addAlertNotificationProperties,
findNextEnabledConfigurationTab,
findPreviousEnabledConfigurationTab,
} from ".";
@@ -748,7 +749,7 @@ describe("Step 7 Theme fallback", () => {
expect(mocks.dispatch.mock.invocationCallOrder.at(-1)).toBeLessThan(
mocks.flushStateToDb.mock.invocationCallOrder[0]
);
- expect(mocks.jumpToStep).toHaveBeenCalledWith(5);
+ expect(mocks.jumpToStep).toHaveBeenCalledWith(5, true);
});
it("loads installed and newly selected Add Service context while
recommending only new services", async () => {
@@ -789,3 +790,23 @@ describe("Step 7 Theme fallback", () => {
});
});
});
+
+describe("addAlertNotificationProperties", () => {
+ it("marks every visible MISC > Notifications field as optional by default",
() => {
+ const result = addAlertNotificationProperties("clusterCreation", {});
+
+ const notificationProperties = result.MISC.Notifications.properties;
+ const visibleFieldNames = ["mail.smtp.host", "mail.smtp.port",
"mail.smtp.from", "ambari.dispatch.recipients"];
+
+ visibleFieldNames.forEach((propertyName) => {
+
expect(notificationProperties[propertyName].propertyAttributes.empty_value_valid).toBe(true);
+ expect(notificationProperties[propertyName].value).toBe("");
+ });
+ });
+
+ it("does not add a Notifications category for the add service wizard", () =>
{
+ const result = addAlertNotificationProperties("addService", {});
+
+ expect(result.MISC).toBeUndefined();
+ });
+});
diff --git a/ambari-web/latest/src/screens/ClusterWizard/Step7/index.tsx
b/ambari-web/latest/src/screens/ClusterWizard/Step7/index.tsx
index 720bb43842..c9d2448fc7 100644
--- a/ambari-web/latest/src/screens/ClusterWizard/Step7/index.tsx
+++ b/ambari-web/latest/src/screens/ClusterWizard/Step7/index.tsx
@@ -167,6 +167,72 @@ export const findInitialConfigurationTab = (disabledTabs:
string[]) => {
return "allConfigurations";
};
+/**
+ * Add alert notification properties
+ * Note: In add service wizard, notification properties should not be added
(matching Ember.js behavior)
+ */
+export const addAlertNotificationProperties = (
+ wizardName: string,
+ updatedConfigProperties: ConfigPropertiesType,
+): ConfigPropertiesType => {
+ // Skip adding notification properties in add service wizard
+ // This matches Ember.js behavior where Notifications category is removed
from MISC in addServiceController
+ if (wizardName === "addService") {
+ return updatedConfigProperties;
+ }
+
+ // Create a deep clone to avoid modifying the original
+ const result = cloneDeep(updatedConfigProperties);
+
+ alert_notifications.forEach((property) => {
+ const {
+ serviceName,
+ name,
+ category,
+ displayName,
+ displayType,
+ filename,
+ isVisible,
+ isRequired,
+ } = property;
+
+ if (!result[serviceName]) {
+ result[serviceName] = {};
+ }
+ if (!result[serviceName][category]) {
+ result[serviceName][category] = {
+ errors: 0,
+ properties: {},
+ };
+ }
+
+ if (isVisible) {
+ result[serviceName][category].properties[name] = {
+ propertyName: name,
+ propertyDisplayname: displayName || "",
+ propertyValue: "",
+ propertyAttributes: {
+ type: displayType || "string",
+ overridable: false,
+ // Mirrors classic's alert_notification.js: every Notifications
field is
+ // optional until notification_configs_view.js's opt-in toggle flips
it
+ // (that toggle isn't ported here, so these stay permanently
optional).
+ empty_value_valid: !isRequired,
+ },
+ previousValue: "",
+ value: displayType === "checkbox" ? "false" : "",
+ final: "false",
+ savedFinal: "false",
+ fileName: filename,
+ type: filename,
+ isEditable: true,
+ };
+ }
+ });
+
+ return result;
+};
+
const preserveEditedConfigValues = (
nextConfigs: ConfigPropertiesType,
currentConfigs: ConfigPropertiesType,
@@ -1569,66 +1635,6 @@ export default function Step7({ wizardName =
"clusterCreation" }: PropTypes) {
return result;
};
- /**
- * Add alert notification properties
- * Note: In add service wizard, notification properties should not be added
(matching Ember.js behavior)
- */
- const addAlertNotificationProperties = (
- updatedConfigProperties: ConfigPropertiesType
- ) => {
- // Skip adding notification properties in add service wizard
- // This matches Ember.js behavior where Notifications category is removed
from MISC in addServiceController
- if (wizardName === "addService") {
- return updatedConfigProperties;
- }
-
- // Create a deep clone to avoid modifying the original
- const result = cloneDeep(updatedConfigProperties);
-
- alert_notifications.forEach((property) => {
- const {
- serviceName,
- name,
- category,
- displayName,
- displayType,
- filename,
- isVisible,
- } = property;
-
- if (!result[serviceName]) {
- result[serviceName] = {};
- }
- if (!result[serviceName][category]) {
- result[serviceName][category] = {
- errors: 0,
- properties: {},
- };
- }
-
- if (isVisible) {
- result[serviceName][category].properties[name] = {
- propertyName: name,
- propertyDisplayname: displayName || "",
- propertyValue: "",
- propertyAttributes: {
- type: displayType || "string",
- overridable: false,
- },
- previousValue: "",
- value: displayType === "checkbox" ? "false" : "",
- final: "false",
- savedFinal: "false",
- fileName: filename,
- type: filename,
- isEditable: true,
- };
- }
- });
-
- return result;
- };
-
/**
* Process stack level configurations
*/
@@ -2327,7 +2333,7 @@ export default function Step7({ wizardName =
"clusterCreation" }: PropTypes) {
updatedConfigProperties =
addServiceConfigCategories(configPropertiesCopy, updatedConfigProperties);
updatedConfigProperties =
organizePropertiesByCategories(configPropertiesCopy, updatedConfigProperties);
updatedConfigProperties = addRemainingProperties(configPropertiesCopy,
updatedConfigProperties);
- updatedConfigProperties =
addAlertNotificationProperties(updatedConfigProperties);
+ updatedConfigProperties = addAlertNotificationProperties(wizardName,
updatedConfigProperties);
updatedConfigProperties = await
processStackLevelConfigurations(updatedConfigProperties);
updatedConfigProperties = onLoadOverrides(updatedConfigProperties);
updatedConfigProperties = initializeValues(updatedConfigProperties);
@@ -2526,7 +2532,7 @@ export default function Step7({ wizardName =
"clusterCreation" }: PropTypes) {
if (wizardName === "addService") {
const nextStep = nextAddServiceStep(4, addServiceFlow);
await Promise.resolve(flushStateToDb("jump", nextStep));
- jumpToStep(nextStep);
+ jumpToStep(nextStep, true);
} else {
await Promise.resolve(flushStateToDb("next"));
handleNextImperitive();
diff --git
a/ambari-web/latest/src/screens/CommonConfigs/ConfigUtils.theme.test.ts
b/ambari-web/latest/src/screens/CommonConfigs/ConfigUtils.theme.test.ts
index 6cd934f446..19540d0974 100644
--- a/ambari-web/latest/src/screens/CommonConfigs/ConfigUtils.theme.test.ts
+++ b/ambari-web/latest/src/screens/CommonConfigs/ConfigUtils.theme.test.ts
@@ -23,9 +23,11 @@ import {
getThemePlacementProperty,
setTabErrorCounts,
updateVisibilityForDependsOn,
+ validateInput,
} from "./ConfigUtils";
import { ConfigPropertiesType } from "./types";
import { normalizeDefaultThemeResponse } from "./themeEngine";
+import { alert_notifications } from "../../data/configs/alert_notifications";
type Placement = {
config: string;
@@ -606,3 +608,60 @@ describe("Service Theme config visibility", () => {
);
});
});
+
+describe("MISC > Notifications properties default to optional (matches
classic's untoggled default)", () => {
+ // Mirrors ambari-web/classic/app/data/configs/alert_notification.js, where
every field is
+ // hardcoded isRequired:false and notification_configs_view.js only flips
that when the user
+ // opts in via createNotification (not ported here) - see
data/configs/alert_notifications.ts.
+ const visibleNotificationFields = alert_notifications.filter(
+ (property) => property.isVisible,
+ );
+
+ it("has at least one visible SMTP/notification field to guard", () => {
+ expect(visibleNotificationFields.length).toBeGreaterThan(0);
+ });
+
+ it.each(visibleNotificationFields.map((property) => [property.name,
property]))(
+ "%s is not required when left empty",
+ (_name, property: any) => {
+ const wizardProperty = {
+ propertyName: property.name,
+ propertyDisplayname: property.displayName || "",
+ propertyValue: "",
+ previousValue: "",
+ value: property.displayType === "checkbox" ? "false" : "",
+ isVisible: true,
+ isHidden: false,
+ isEditable: true,
+ propertyAttributes: {
+ type: property.displayType || "string",
+ overridable: false,
+ empty_value_valid: !property.isRequired,
+ },
+ };
+
+ expect(validateInput(wizardProperty, "")).toBe("");
+ },
+ );
+
+ it("would have blocked the wizard before empty_value_valid was wired up from
isRequired", () => {
+ const propertyMissingEmptyValueValid = {
+ propertyName: "mail.smtp.host",
+ propertyDisplayname: "SMTP Host",
+ propertyValue: "",
+ previousValue: "",
+ value: "",
+ isVisible: true,
+ isHidden: false,
+ isEditable: true,
+ propertyAttributes: {
+ type: "host",
+ overridable: false,
+ },
+ };
+
+ expect(validateInput(propertyMissingEmptyValueValid, "")).toBe(
+ "This is required",
+ );
+ });
+});
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
