Skip to content

Fix: use different query for add-on ddl #303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
santipalenque wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix: use different query for add-on ddl #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cdf7c62
chore: use different query for add-on ddl
santipalenque Jul 31, 2026
026ed56
v5.0.48-beta.0
santipalenque Aug 3, 2026
eba7583
chore: pr review
santipalenque Aug 6, 2026
df30124
v5.0.48-beta.1
santipalenque Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "openstack-uicore-foundation",
"version": "5.0.44",
"version": "5.0.48-beta.1",
"description": "ui reactjs components for openstack marketing site",
"main": "lib/openstack-uicore-foundation.js",
"scripts": {
Expand Down
4 changes: 2 additions & 2 deletions src/components/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export {default as MuiMenuButton} from './mui/menu-button'
export {default as MuiSearchInput} from './mui/search-input'
export {default as MuiShowConfirmDialog} from './mui/showConfirmDialog'
export {default as MuiSponsorAddonSelect} from './mui/sponsor-addon-select'
export {default as MuiSummitAddonSelect} from './mui/summit-addon-select'
export {default as MuiAddonTypeSelect} from './mui/addon-type-select'
export {default as MuiSummitsDropdown} from './mui/summits-dropdown'
export {default as MuiFormItemTable, getCurrentApplicableRate, isItemAvailable, GlobalQuantityField, ItemTableField, UnderlyingAlertNote, ExpandedRowContent} from './mui/FormItemTable'
export {default as MuiItemSettingsModal} from './mui/ItemSettingsModal'
Expand Down Expand Up @@ -96,7 +96,7 @@ export {default as MuiFormikRadioGroup} from './mui/formik-inputs/mui-formik-rad
export {default as MuiFormikSelectGroup} from './mui/formik-inputs/mui-formik-select-group'
export {default as MuiFormikSelect} from './mui/formik-inputs/mui-formik-select'
export {default as MuiFormikSelectV2} from './mui/formik-inputs/mui-formik-select-v2'
export {default as MuiFormikSummitAddonSelect} from './mui/formik-inputs/mui-formik-summit-addon-select'
export {default as MuiFormikAddonTypeSelect} from './mui/formik-inputs/mui-formik-addon-type-select'
export {default as MuiFormikSwitch} from './mui/formik-inputs/mui-formik-switch'
export {default as MuiFormikTextField} from './mui/formik-inputs/mui-formik-textfield'
export {default as MuiFormikTextEditor} from './mui/formik-inputs/mui-formik-text-editor'
Expand Down
22 changes: 12 additions & 10 deletions ...mui/__tests__/summit-addon-select.test.js → ...s/mui/__tests__/addon-type-select.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,44 @@
* */

jest.mock("../../../utils/query-actions", () => ({
querySummitAddons: jest.fn((summitId, callback) => {
callback(["Addon Alpha", "Addon Beta"]);
querySummitAddons: jest.fn((callback) => {
callback([
{ id: 1, name: "Addon Alpha" },
{ id: 2, name: "Addon Beta" }
]);
})
}));

import React from "react";
import { render, screen, waitFor } from "@testing-library/react";
import "@testing-library/jest-dom";
import SummitAddonSelect from "../summit-addon-select";
import AddonTypeSelect from "../addon-type-select";

describe("SummitAddonSelect", () => {
describe("AddonTypeSelect", () => {
const defaultProps = {
value: "",
summitId: 5,
onChange: jest.fn(),
placeholder: "Select addon..."
};

beforeEach(() => jest.clearAllMocks());

test("renders a select combobox", () => {
const { container } = render(<SummitAddonSelect {...defaultProps} />);
const { container } = render(<AddonTypeSelect {...defaultProps} />);
expect(container.querySelector("[role='combobox']")).toBeInTheDocument();
});

test("calls querySummitAddons on mount with summitId", async () => {
render(<SummitAddonSelect {...defaultProps} />);
test("calls querySummitAddons on mount", async () => {
render(<AddonTypeSelect {...defaultProps} />);
await waitFor(() => {
expect(
require("../../../utils/query-actions").querySummitAddons
).toHaveBeenCalledWith(5, expect.any(Function));
).toHaveBeenCalledWith(expect.any(Function));
});
});

test("renders options returned by querySummitAddons", async () => {
render(<SummitAddonSelect {...defaultProps} />);
render(<AddonTypeSelect {...defaultProps} />);
// Options are rendered inside the Select's listbox - click to open
// Just verify the component renders without errors after options load
await waitFor(() => {
Expand Down
22 changes: 11 additions & 11 deletions ...__/mui-formik-summit-addon-select.test.js → ...ts__/mui-formik-addon-type-select.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
* limitations under the License.
* */

jest.mock("../summit-addon-select", () => {
jest.mock("../addon-type-select", () => {
const React = require("react");
return {
__esModule: true,
default: ({ value, placeholder, inputProps }) => (
<div
data-testid="summit-addon-select"
data-testid="addon-type-select"
data-value={value}
data-placeholder={placeholder}
data-error={inputProps?.error ? "true" : "false"}
Expand All @@ -32,24 +32,24 @@ import React from "react";
import { render, screen } from "@testing-library/react";
import { Formik, Form } from "formik";
import "@testing-library/jest-dom";
import MuiFormikSummitAddonSelect from "../formik-inputs/mui-formik-summit-addon-select";
import MuiFormikAddonTypeSelect from "../formik-inputs/mui-formik-addon-type-select";

const renderWithFormik = (props, initialValues = { addon: "" }) =>
render(
<Formik initialValues={initialValues} onSubmit={jest.fn()}>
<Form>
<MuiFormikSummitAddonSelect name="addon" summitId={1} {...props} />
<MuiFormikAddonTypeSelect name="addon" {...props} />
</Form>
</Formik>
);

describe("MuiFormikSummitAddonSelect", () => {
test("renders the SummitAddonSelect", () => {
describe("MuiFormikAddonTypeSelect", () => {
test("renders the AddonTypeSelect", () => {
renderWithFormik({});
expect(screen.getByTestId("summit-addon-select")).toBeInTheDocument();
expect(screen.getByTestId("addon-type-select")).toBeInTheDocument();
});

test("passes placeholder to SummitAddonSelect", () => {
test("passes placeholder to AddonTypeSelect", () => {
renderWithFormik({ placeholder: "Choose addon" });
expect(screen.getByText("Choose addon")).toBeInTheDocument();
});
Expand All @@ -63,19 +63,19 @@ describe("MuiFormikSummitAddonSelect", () => {
onSubmit={jest.fn()}
>
<Form>
<MuiFormikSummitAddonSelect name="addon" summitId={1} />
<MuiFormikAddonTypeSelect name="addon" />
</Form>
</Formik>
);
expect(screen.getByTestId("summit-addon-select")).toHaveAttribute(
expect(screen.getByTestId("addon-type-select")).toHaveAttribute(
"data-error",
"true"
);
});

test("passes false error when not touched", () => {
renderWithFormik({});
expect(screen.getByTestId("summit-addon-select")).toHaveAttribute(
expect(screen.getByTestId("addon-type-select")).toHaveAttribute(
"data-error",
"false"
);
Expand Down
14 changes: 6 additions & 8 deletions src/components/mui/summit-addon-select.js → src/components/mui/addon-type-select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,19 @@ import { MenuItem, Select } from "@mui/material";
import PropTypes from "prop-types";
import { querySummitAddons } from "../../utils/query-actions";

const SummitAddonSelect = ({
const AddonTypeSelect = ({
value,
summitId,
placeholder = "Select...",
onChange,
inputProps = {}
}) => {
const [options, setOptions] = useState([]);

useEffect(() => {
querySummitAddons(summitId, (results) => {
querySummitAddons((results) => {
const normalized = results.map((r) => ({
value: r,
label: r
value: r.name,
label: r.name
Comment on lines 29 to +31

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 5 \
  '\b(MuiFormikAddonTypeSelect|MuiSummitAddonSelect|MuiFormikSummitAddonSelect|AddonTypeSelect)\b' \
  --glob '*.{js,jsx,mjs,cjs}' . || true

rg -n -C 5 \
  '\b(initialValues|setFieldValue|setValue|addon)\b' \
  --glob '*.{js,jsx,mjs,cjs}' . || true

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 50397

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
find . -path './node_modules' -prune -o -path './dist' -prune -o -path './build' -prune -o \( -type f \( -name '*.js' -o -name '*.jsx' -o -name '*.mjs' -o -name '*.cjs' \) \) -print | sed 's#^\./##' | sort

echo "== component slices =="
sed -n '1,120p' src/components/mui/addon-type-select.js
echo "--- formik wrapper ---"
sed -n '1,80p' src/components/mui/formik-inputs/mui-formik-addon-type-select.js
echo "--- mui index slice ---"
sed -n '60,105p' src/components/index.js
echo "--- direct imports/usages (focused) =="
rg -n --glob '*.{js,jsx,mjs,cjs}' 'from ["'\''].*addon-type-select["'\'']|<MuiFormikAddonTypeSelect|<MuiAddonTypeSelect' . || true

echo "== addon field occurrences (focused) =="
rg -n --glob '*.{js,jsx,mjs,cjs}' '\badd[on]s?\b' . | head -n 220

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 25887

Migrate the Formik value contract before using addon names.

value: r.name changes the selector value from the full addon object to a string. MuiFormikAddonTypeSelect forwards field.value unchanged and stores the selected value directly. If an existing form still initializes addon with the previous object shape, no MenuItem matches it.

Use addon names for existing Formik initial values, persisted values, and submit payloads, or normalize legacy values at the Formik boundary. Add a regression test for the old value shape.

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

In `@src/components/mui/addon-type-select.js` around lines 29 - 31, Update the
Formik contract used by MuiFormikAddonTypeSelect so addon values are
consistently represented by names across initial values, persisted data, and
submit payloads, or normalize legacy addon objects at the Formik boundary before
selection. Ensure existing object-shaped values still resolve to the matching
MenuItem, and add a regression test covering the legacy value shape.

}));
setOptions(normalized);
});
Expand Down Expand Up @@ -64,11 +63,10 @@ const SummitAddonSelect = ({
);
};

SummitAddonSelect.propTypes = {
AddonTypeSelect.propTypes = {
value: PropTypes.string,
summitId: PropTypes.number.isRequired,
placeholder: PropTypes.string,
onChange: PropTypes.func.isRequired
};

export default SummitAddonSelect;
export default AddonTypeSelect;
13 changes: 5 additions & 8 deletions ...-inputs/mui-formik-summit-addon-select.js → ...ik-inputs/mui-formik-addon-type-select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,18 @@
import React from "react";
import PropTypes from "prop-types";
import { useField } from "formik";
import SummitAddonSelect from "../summit-addon-select";
import AddonTypeSelect from "../addon-type-select";

const MuiFormikSummitAddonSelect = ({
const MuiFormikAddonTypeSelect = ({
name,
summitId,
placeholder = "Select...",
inputProps = {}
}) => {
const [field, meta, helpers] = useField(name);

return (
<SummitAddonSelect
<AddonTypeSelect
value={field.value || ""}
summitId={summitId}
onChange={helpers.setValue}
placeholder={placeholder}
inputProps={{
Expand All @@ -39,11 +37,10 @@ const MuiFormikSummitAddonSelect = ({
);
};

MuiFormikSummitAddonSelect.propTypes = {
MuiFormikAddonTypeSelect.propTypes = {
name: PropTypes.string.isRequired,
summitId: PropTypes.number.isRequired,
placeholder: PropTypes.string,
inputProps: PropTypes.object
};

export default MuiFormikSummitAddonSelect;
export default MuiFormikAddonTypeSelect;
4 changes: 2 additions & 2 deletions src/utils/query-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,14 +631,14 @@ export const querySponsorAddons = async (


export const querySummitAddons = async (
summitId,
callback
) => {
const endpoint = URI(
`/api/v1/summits/${summitId}/add-ons/metadata`
`/api/v1/summits/all/add-on-types`
Comment thread
santipalenque marked this conversation as resolved.
);
endpoint.addQuery("page", 1);
endpoint.addQuery("per_page", MAX_PER_PAGE);
Comment thread
santipalenque marked this conversation as resolved.
endpoint.addQuery("order", "+name");

_fetch(endpoint, callback);
};
Expand Down
4 changes: 2 additions & 2 deletions webpack.common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ module.exports = {
'components/mui/search-input': './src/components/mui/search-input.js',
'components/mui/show-confirm-dialog': './src/components/mui/showConfirmDialog.js',
'components/mui/sponsor-addon-select': './src/components/mui/sponsor-addon-select.js',
'components/mui/summit-addon-select': './src/components/mui/summit-addon-select.js',
'components/mui/addon-type-select': './src/components/mui/addon-type-select.js',
'components/mui/summits-dropdown': './src/components/mui/summits-dropdown.js',
'components/mui/form-item-table': './src/components/mui/FormItemTable/index.js',
'components/mui/item-settings-modal': './src/components/mui/ItemSettingsModal/index.js',
Expand Down Expand Up @@ -126,7 +126,7 @@ module.exports = {
'components/mui/formik-inputs/select-group': './src/components/mui/formik-inputs/mui-formik-select-group.js',
'components/mui/formik-inputs/select': './src/components/mui/formik-inputs/mui-formik-select.js',
'components/mui/formik-inputs/select-v2': './src/components/mui/formik-inputs/mui-formik-select-v2.js',
'components/mui/formik-inputs/summit-addon-select': './src/components/mui/formik-inputs/mui-formik-summit-addon-select.js',
'components/mui/formik-inputs/addon-type-select': './src/components/mui/formik-inputs/mui-formik-addon-type-select.js',
'components/mui/formik-inputs/switch': './src/components/mui/formik-inputs/mui-formik-switch.js',
'components/mui/formik-inputs/textfield': './src/components/mui/formik-inputs/mui-formik-textfield.js',
'components/mui/formik-inputs/texteditor': './src/components/mui/formik-inputs/mui-formik-text-editor.js',
Expand Down
Loading