Skip to content

Commit

Permalink
Fixes #37013 - change the 'All hosts' menu item's url based on the `n…
Browse files Browse the repository at this point in the history
…ew_host_page` setting

After changing the `new_host_page` setting, the layout navigation still has the wrong hosts page link (the one it got initially)
and someone who wants to try this new functionallity can't access the page via the navigation.

Using ForemanContext, in this PR we are updating the menu item based on the changed setting.
  • Loading branch information
ronlavi2412 authored and jeremylenz committed Jan 16, 2024
1 parent 20798d2 commit 0b40a64
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 16 deletions.
2 changes: 1 addition & 1 deletion app/registries/foreman/settings/general.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
full_name: N_('Show Experimental Labs'))
setting('new_hosts_page',
type: :boolean,
description: N_("Whether or not to show the new overview page for All Hosts (requires reload of page)"),
description: N_("Whether or not to show the new overview page for All Hosts"),
default: false,
full_name: N_('Show New Host Overview Page'))
setting('display_fqdn_for_hosts',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import forceSingleton from '../../common/forceSingleton';

export const getForemanContext = contextData =>
forceSingleton('Context', () => React.createContext(contextData));
export const useForemanContext = () => React.useContext(getForemanContext());
export const useForemanContext = () =>
React.useContext(getForemanContext())?.context;
export const useForemanSetContext = () =>
React.useContext(getForemanContext())?.setContext;

const useForemanMetadata = () => useForemanContext()?.metadata || {};

Expand All @@ -14,7 +17,10 @@ export const useForemanOrganization = () => useForemanMetadata().organization;
export const useForemanLocation = () => useForemanMetadata().location;
export const useForemanUser = () => useForemanMetadata().user;

export const getHostsPageUrl = displayNewHostsPage =>
displayNewHostsPage ? '/new/hosts' : '/hosts';

export const useForemanHostsPageUrl = () => {
const { displayNewHostsPage } = useForemanSettings();
return displayNewHostsPage ? '/new/hosts' : '/hosts';
return getHostsPageUrl(displayNewHostsPage);
};
5 changes: 3 additions & 2 deletions webpack/assets/javascripts/react_app/Root/ReactApp.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useState } from 'react';
import PropTypes from 'prop-types';
import { ConnectedRouter } from 'connected-react-router';
import { ApolloProvider } from '@apollo/client';
Expand All @@ -13,7 +13,8 @@ import ErrorBoundary from '../components/common/ErrorBoundary';
import ConfirmModal from '../components/ConfirmModal';

const ReactApp = ({ layout, metadata, toasts }) => {
const contextData = { metadata };
const [context, setContext] = useState({ metadata });
const contextData = { context, setContext };
const ForemanContext = getForemanContext(contextData);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export const LAYOUT_SHOW_LOADING = 'LAYOUT_SHOW_LOADING';
export const LAYOUT_HIDE_LOADING = 'LAYOUT_HIDE_LOADING';
export const LAYOUT_COLLAPSE = 'LAYOUT_COLLAPSE';
export const LAYOUT_EXPAND = 'LAYOUT_EXPAND';
export const NAV_MENU_ALL_HOST = 'All Hosts';
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
noop,
foremanUrl,
} from '../../common/helpers';
import { NAV_MENU_ALL_HOST } from './LayoutConstants';
import { getHostsPageUrl } from '../../Root/Context/ForemanContext';

export const createInitialTaxonomy = (currentTaxonomy, availableTaxonomies) => {
const taxonomyId = availableTaxonomies.find(
Expand All @@ -22,14 +24,20 @@ export const createInitialTaxonomy = (currentTaxonomy, availableTaxonomies) => {
export const getCurrentPath = () =>
removeLastSlashFromPath(window.location.pathname);

export const combineMenuItems = data => {
export const combineMenuItems = (data, displayNewHostsPage) => {
const items = [];

data.menu.forEach(item => {
const translatedChildren = item.children.map(child => ({
...child,
title: isEmpty(child.name) ? child.name : __(child.name),
}));
const translatedChildren = item.children.map(child => {
const newChild = {
...child,
title: isEmpty(child.name) ? child.name : __(child.name),
};
if (child.name === NAV_MENU_ALL_HOST) {
newChild.url = getHostsPageUrl(displayNewHostsPage);
}
return newChild;
});

const translatedItem = {
...item,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const Navigation = ({
return { ...rest, groups };
}),
// eslint-disable-next-line react-hooks/exhaustive-deps
[items.length, currentPath]
[items, currentPath]
);

const [currentExpanded, setCurrentExpanded] = useState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,27 @@ import { getIsNavbarCollapsed } from './LayoutSessionStorage';
import {
useForemanOrganization,
useForemanLocation,
useForemanSettings,
} from '../../Root/Context/ForemanContext';

import Layout from './Layout';

const ConnectedLayout = ({ children, data }) => {
const dispatch = useDispatch();
const { displayNewHostsPage } = useForemanSettings();

const currentLocation = useForemanLocation()?.title;
const currentOrganization = useForemanOrganization()?.title;
useEffect(() => {
dispatch(
initializeLayout({
items: combineMenuItems(data),
items: combineMenuItems(data, displayNewHostsPage),
isCollapsed: getIsNavbarCollapsed(),
organization: data.orgs.current_org,
location: data.locations.current_location,
})
);
}, [data, dispatch]);
}, [data, dispatch, displayNewHostsPage]);

const isNavCollapsed = useSelector(state => selectIsCollapsed(state));
useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export const SETTING_UPDATE_MODAL = 'settingUpdateModal';
export const SETTING_UPDATE_PATH = '/api/settings/:id';
export const SETTING_NEW_HOSTS_PAGE = 'new_hosts_page';
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,22 @@ import PropTypes from 'prop-types';
import { Field as FormikField } from 'formik';
import ForemanForm from '../../../common/forms/ForemanForm';
import SettingValueField from '../SettingValueField';
import { SETTING_UPDATE_PATH } from '../../SettingUpdateModalConstants';
import {
SETTING_NEW_HOSTS_PAGE,
SETTING_UPDATE_PATH,
} from '../../SettingUpdateModalConstants';

import { translate as __ } from '../../../../common/I18n';
import { useForemanSetContext } from '../../../../Root/Context/ForemanContext';

const SettingForm = ({
setting,
initialValues,
setModalClosed,
submitForm,
}) => {
const setContext = useForemanSetContext();

const handleSubmit = (values, actions) => {
let submitValues = { setting: values };

Expand All @@ -23,14 +29,23 @@ const SettingForm = ({
: values.value.split(',').map(item => item.trim());
submitValues = { setting: { value: splitValue } };
}

let successCallback = setModalClosed;
if (setting.name === SETTING_NEW_HOSTS_PAGE) {
const value = values.value === 'true';
successCallback = () =>
setContext(context => {
context.metadata.UISettings.displayNewHostsPage = value;
setModalClosed();
return context;
});
}
return submitForm({
url: SETTING_UPDATE_PATH.replace(':id', setting.id),
values: submitValues,
item: 'Settings',
message: __('Setting updated.'),
method: 'put',
successCallback: setModalClosed,
successCallback,
errorToast: error =>
`${__('Error updating setting')}: ${error.response?.data?.error
?.message ?? error.message}`,
Expand Down
2 changes: 2 additions & 0 deletions webpack/test_setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ jest.mock('./assets/javascripts/react_app/Root/Context/ForemanContext', () => ({
useForemanDocUrl: () => '/url',
useForemanLocation: () => ({ title: 'location' }),
useForemanOrganization: () => ({ title: 'organization' }),
getHostsPageUrl: displayNewHostsPage =>
displayNewHostsPage ? '/new/hosts' : '/hosts',
}));
jest.mock('./assets/javascripts/react_app/common/I18n');
jest.mock('./assets/javascripts/foreman_tools', () => ({
Expand Down

0 comments on commit 0b40a64

Please sign in to comment.