From 13b047ac3016774787f69d20a8150c8e8eab7180 Mon Sep 17 00:00:00 2001 From: Michael Ro Date: Fri, 16 Nov 2018 13:52:22 -0500 Subject: [PATCH] Merge pull request #769 from mzazrivec/fix_vertical_menu Fix vertical navigation menu for v2v (cherry picked from commit 8957e39261ec1057433d8887b6da3342cbd8f0c3) https://bugzilla.redhat.com/show_bug.cgi?id=1642104 --- app/controllers/migration_controller.rb | 10 ++++++++-- app/javascript/react/config/Routes.js | 4 ++-- app/javascript/react/config/config.js | 8 ++++---- app/javascript/react/index.js | 6 +++--- .../react/screens/App/Mappings/Mappings.js | 14 +++++++------- .../screens/App/Mappings/MappingsConstants.js | 2 +- .../MappingWizardDatastoresStepConstants.js | 6 +++--- .../MappingWizardNetworksStepConstants.js | 6 +++--- .../MappingWizardResultsStep.js | 4 ++-- .../react/screens/App/Overview/Overview.js | 2 +- .../Migrations/InProgressWithDetailCard.js | 2 +- .../Migrations/MigrationsCompletedList.js | 2 +- .../Migrations/MigrationsNotStartedList.js | 2 +- .../__test__/MigrationsNotStartedList.test.js | 2 +- .../PlanWizardInstancePropertiesStep.js | 2 +- app/javascript/react/screens/App/Plan/Plan.js | 3 +-- .../common/NotificationList/NotificationList.js | 3 +-- app/javascript/redux/actions/routerActions.js | 9 ++------- config/routes.rb | 5 ++++- lib/manageiq/v2v/engine.rb | 6 +++--- 20 files changed, 50 insertions(+), 48 deletions(-) diff --git a/app/controllers/migration_controller.rb b/app/controllers/migration_controller.rb index 9383b20d4b..465b5c674f 100644 --- a/app/controllers/migration_controller.rb +++ b/app/controllers/migration_controller.rb @@ -5,8 +5,14 @@ class MigrationController < ApplicationController after_action :cleanup_action def index - # this sets the active menu item, must match the item name in lib/manageiq-v2v/engine.rb - @layout = 'migration' + @layout = case request.path + when '/migration/overview', /^\/migration\/plan\/.*/ + 'overview' + when '/migration/mappings' + 'mappings' + when '/migration/settings' + 'settings' + end @page_title = _('Migration') end diff --git a/app/javascript/react/config/Routes.js b/app/javascript/react/config/Routes.js index 4f6ecce4a9..ee6ab92b52 100644 --- a/app/javascript/react/config/Routes.js +++ b/app/javascript/react/config/Routes.js @@ -14,7 +14,7 @@ const Routes = ({ store }) => { if (props.match.isExact) { return ( @@ -28,7 +28,7 @@ const Routes = ({ store }) => /> ); } - return ; + return ; }); Routes.propTypes = { diff --git a/app/javascript/react/config/config.js b/app/javascript/react/config/config.js index f2629dc00d..4caaf3c90f 100644 --- a/app/javascript/react/config/config.js +++ b/app/javascript/react/config/config.js @@ -5,19 +5,19 @@ import MappingsContainer from '../screens/App/Mappings'; export const links = [ { - path: '', + path: '/migration/overview', component: OverviewContainer }, { - path: 'settings', + path: '/migration/settings', component: Settings }, { - path: 'plan/:id', + path: '/migration/plan/:id', component: PlanContainer }, { - path: 'mappings', + path: '/migration/mappings', component: MappingsContainer } ]; diff --git a/app/javascript/react/index.js b/app/javascript/react/index.js index f0012e9543..36e1b7d19d 100644 --- a/app/javascript/react/index.js +++ b/app/javascript/react/index.js @@ -1,5 +1,5 @@ import React from 'react'; -import { ConnectedRouter } from 'connected-react-router'; +import { BrowserRouter } from 'react-router-dom'; import { connect } from 'react-redux'; import { Spinner } from 'patternfly-react'; import PropTypes from 'prop-types'; @@ -21,12 +21,12 @@ class App extends React.Component { ); return ( - + - + ); } } diff --git a/app/javascript/react/screens/App/Mappings/Mappings.js b/app/javascript/react/screens/App/Mappings/Mappings.js index cb0ce78a73..80e6ee22bd 100644 --- a/app/javascript/react/screens/App/Mappings/Mappings.js +++ b/app/javascript/react/screens/App/Mappings/Mappings.js @@ -165,7 +165,7 @@ class Mappings extends Component { {__('Compute')} - {__('Migration')} + {__('Migration')} {__('Infrastructure Mappings')} { this.props.continueToPlanAction(id); - this.props.redirectTo('/'); + this.props.redirectTo('/migration/overview'); }; render() { @@ -118,7 +118,7 @@ MappingWizardResultsStep.propTypes = { redirectTo: PropTypes.func }; MappingWizardResultsStep.defaultProps = { - postMappingsUrl: 'api/transformation_mappings', + postMappingsUrl: '/api/transformation_mappings', postTransformMappingsAction: noop, transformationsBody: {}, isPostingMappings: true, diff --git a/app/javascript/react/screens/App/Overview/Overview.js b/app/javascript/react/screens/App/Overview/Overview.js index 524e2f7b43..cda1ff7834 100644 --- a/app/javascript/react/screens/App/Overview/Overview.js +++ b/app/javascript/react/screens/App/Overview/Overview.js @@ -328,7 +328,7 @@ class Overview extends React.Component { description={__('Create an infrastructure mapping to later be used by a migration plan')} buttonText={__('Create Infrastructure Mapping')} showWizardAction={() => { - this.redirectTo('/mappings'); + this.redirectTo('/migration/mappings'); openMappingWizardOnTransitionAction(); }} className="full-page-empty" diff --git a/app/javascript/react/screens/App/Overview/components/Migrations/InProgressWithDetailCard.js b/app/javascript/react/screens/App/Overview/components/Migrations/InProgressWithDetailCard.js index 1e3b3d1f27..5ec81c4bcc 100644 --- a/app/javascript/react/screens/App/Overview/components/Migrations/InProgressWithDetailCard.js +++ b/app/javascript/react/screens/App/Overview/components/Migrations/InProgressWithDetailCard.js @@ -7,7 +7,7 @@ const InProgressWithDetailCard = ({ plan, failedOverlay, handleClick, children } { if (!e.target.classList.contains('pficon-error-circle-o')) { - handleClick(`/plan/${plan.id}`); + handleClick(`/migration/plan/${plan.id}`); } }} className="in-progress" diff --git a/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsCompletedList.js b/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsCompletedList.js index c72a428108..395dd47107 100644 --- a/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsCompletedList.js +++ b/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsCompletedList.js @@ -191,7 +191,7 @@ const MigrationsCompletedList = ({ onClick={e => { e.stopPropagation(); - redirectTo(`/plan/${plan.id}`); + redirectTo(`/migration/plan/${plan.id}`); }} key={plan.id} leftContent={ diff --git a/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsNotStartedList.js b/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsNotStartedList.js index c83dc11d48..1247a41ab4 100644 --- a/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsNotStartedList.js +++ b/app/javascript/react/screens/App/Overview/components/Migrations/MigrationsNotStartedList.js @@ -87,7 +87,7 @@ const MigrationsNotStartedList = ({ stacked className="plans-not-started-list__list-item" onClick={() => { - redirectTo(`/plan/${plan.id}`); + redirectTo(`/migration/plan/${plan.id}`); }} actions={
diff --git a/app/javascript/react/screens/App/Overview/components/Migrations/__test__/MigrationsNotStartedList.test.js b/app/javascript/react/screens/App/Overview/components/Migrations/__test__/MigrationsNotStartedList.test.js index 7ddab4bfea..1e0801e3ad 100644 --- a/app/javascript/react/screens/App/Overview/components/Migrations/__test__/MigrationsNotStartedList.test.js +++ b/app/javascript/react/screens/App/Overview/components/Migrations/__test__/MigrationsNotStartedList.test.js @@ -29,7 +29,7 @@ test('clicking on a plan fires redirectTo with the path to its details page', () .at(0) .simulate('click'); - expect(redirectTo).toHaveBeenLastCalledWith(`/plan/${notStartedPlan.id}`); + expect(redirectTo).toHaveBeenLastCalledWith(`/migration/plan/${notStartedPlan.id}`); }); test.skip('clicking on the Migrate button fires migrateClick with the correct API endpoint', () => { diff --git a/app/javascript/react/screens/App/Overview/screens/PlanWizard/components/PlanWizardInstancePropertiesStep/PlanWizardInstancePropertiesStep.js b/app/javascript/react/screens/App/Overview/screens/PlanWizard/components/PlanWizardInstancePropertiesStep/PlanWizardInstancePropertiesStep.js index f44a59e1c1..e6ae53a00a 100644 --- a/app/javascript/react/screens/App/Overview/screens/PlanWizard/components/PlanWizardInstancePropertiesStep/PlanWizardInstancePropertiesStep.js +++ b/app/javascript/react/screens/App/Overview/screens/PlanWizard/components/PlanWizardInstancePropertiesStep/PlanWizardInstancePropertiesStep.js @@ -119,7 +119,7 @@ PlanWizardInstancePropertiesStep.propTypes = { PlanWizardInstancePropertiesStep.defaultProps = { fetchOpenstackTenantUrl: '/api/cloud_tenants', - bestFitFlavorUrl: 'api/transformation_mappings', + bestFitFlavorUrl: '/api/transformation_mappings', queryOpenstackTenantAttributes: ['flavors', 'security_groups', 'default_security_group'] }; diff --git a/app/javascript/react/screens/App/Plan/Plan.js b/app/javascript/react/screens/App/Plan/Plan.js index f6e0cef402..b8081a3e0a 100644 --- a/app/javascript/react/screens/App/Plan/Plan.js +++ b/app/javascript/react/screens/App/Plan/Plan.js @@ -1,7 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import Immutable from 'seamless-immutable'; -import { Link } from 'react-router-dom'; import { Breadcrumb, Spinner, Icon } from 'patternfly-react'; import Toolbar from '../../../config/Toolbar'; import PlanRequestDetailList from './components/PlanRequestDetailList/PlanRequestDetailList'; @@ -165,7 +164,7 @@ class Plan extends React.Component { {__('Compute')}
  • - {__('Migration')} + {__('Migration')}
  • {!isRejectedPlan && planName && ( diff --git a/app/javascript/react/screens/App/common/NotificationList/NotificationList.js b/app/javascript/react/screens/App/common/NotificationList/NotificationList.js index 240679afaa..25301ce492 100644 --- a/app/javascript/react/screens/App/common/NotificationList/NotificationList.js +++ b/app/javascript/react/screens/App/common/NotificationList/NotificationList.js @@ -1,7 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { TimedToastNotification, ToastNotificationList } from 'patternfly-react'; -import { Link } from 'react-router-dom'; class NotificationList extends React.Component { render() { @@ -25,7 +24,7 @@ class NotificationList extends React.Component { {notification.actionEnabled && (
    - {__('View Details')} + {__('View Details')}
    )} diff --git a/app/javascript/redux/actions/routerActions.js b/app/javascript/redux/actions/routerActions.js index c272137e6b..d72bc864bd 100644 --- a/app/javascript/redux/actions/routerActions.js +++ b/app/javascript/redux/actions/routerActions.js @@ -1,8 +1,3 @@ -import { push } from 'connected-react-router'; - -export const redirectTo = path => (dispatch, getState) => { - // NOTE: to avoid pushing the same path to history and throwing error - if (getState().router.location.pathname !== path) { - dispatch(push(path)); - } +export const redirectTo = path => () => { + window.DoNav(path); }; diff --git a/config/routes.rb b/config/routes.rb index 14d6086708..eca5803f00 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,9 @@ # ManageIQ::V2V::Engine.routes.draw do Rails.application.routes.draw do - match '/migration' => 'migration#index', :via => [:get] + match '/migration' => redirect('/migration/overview'), :via => [:get] + match '/migration/overview' => 'migration#index', :via => [:get] + match '/migration/mappings' => 'migration#index', :via => [:get] + match '/migration/plan/:id' => 'migration#index', :via => [:get] match 'migration/*page' => 'migration#index', :via => [:get] get "migration_log/download_migration_log(/:id)", diff --git a/lib/manageiq/v2v/engine.rb b/lib/manageiq/v2v/engine.rb index f46de810c2..a34a0efcaf 100644 --- a/lib/manageiq/v2v/engine.rb +++ b/lib/manageiq/v2v/engine.rb @@ -13,9 +13,9 @@ def vmdb_plugin? initializer 'plugin' do Menu::CustomLoader.register( Menu::Section.new(:migration, N_("Migration"), 'fa fa-plus', [ - Menu::Item.new('migration', N_("Overview"), 'migration', {:feature => 'migration', :any => true}, '/migration'), - Menu::Item.new('mappings', N_("Infrastructure Mappings"), 'mappings', {:feature => 'mappings', :any => true}, '/migration#/mappings'), - Menu::Item.new('settings', N_("Migration Settings"), 'migration_settings', {:feature => 'migration_settings', :any => true}, '/migration#/settings') + Menu::Item.new('overview', N_("Overview"), 'migration', {:feature => 'migration', :any => true}, '/migration/overview'), + Menu::Item.new('mappings', N_("Infrastructure Mappings"), 'mappings', {:feature => 'mappings', :any => true}, '/migration/mappings'), + Menu::Item.new('settings', N_("Migration Settings"), 'migration_settings', {:feature => 'migration_settings', :any => true}, '/migration/settings') ], nil, nil, nil, nil, :compute) ) end