Skip to content

Commit

Permalink
Move AppListView route to be cluster-aware. (#1793)
Browse files Browse the repository at this point in the history
* Move AppListView route to be cluster-aware.

First of several PRs which will transition existing routes in the UI to
be cluster-aware, defaulting to the "default" cluster so as to not break
the existing experience.

* Fix app list link from operator deletion

* Add reusable url for app list.

* Updated and simplified use of HeaderLink. Fixed lint.
  • Loading branch information
absoludity committed Jun 19, 2020
1 parent 31f2fd3 commit a0fabad
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 73 deletions.
1 change: 1 addition & 0 deletions dashboard/src/components/AppList/AppList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import CustomResourceListItem from "./CustomResourceListItem";
interface IAppListProps {
apps: IAppState;
fetchAppsWithUpdateInfo: (ns: string, all: boolean) => void;
cluster: string;
namespace: string;
pushSearchFilter: (filter: string) => any;
filter: string;
Expand Down
16 changes: 10 additions & 6 deletions dashboard/src/components/Header/Header.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { shallow } from "enzyme";
import * as React from "react";
import { INamespaceState } from "../../reducers/namespace";
import { app } from "../../shared/url";
import Header from "./Header";


Expand All @@ -24,13 +25,14 @@ it("renders the header links and titles", () => {
const menubar = wrapper.find(".header__nav__menu").first();
const items = menubar.children().map(p => p.props().children.props);
const expectedItems = [
{ children: "Applications", to: "apps" },
{ children: "Catalog", to: "catalog" },
{ children: "Service Instances (alpha)", to: "services/instances" },
{ children: "Applications", to: app.apps.list("default") },
{ children: "Catalog", to: app.catalog("default") },
{ children: "Service Instances (alpha)", to: app.servicesInstances("default") },
];
items.forEach((item, index) => {
expect(item.children).toBe(expectedItems[index].children);
expect(item.to).toBe(expectedItems[index].to);
expect(items.length).toEqual(expectedItems.length);
expectedItems.forEach((expectedItem, index) => {
expect(expectedItem.children).toBe(items[index].children);
expect(expectedItem.to).toBe(items[index].to);
});
});

Expand Down Expand Up @@ -95,6 +97,7 @@ it("call setNamespace and getNamespace when selecting a namespace", () => {
const createNamespace = jest.fn();
const getNamespace = jest.fn();
const namespace = {
cluster: "default",
current: "foo",
namespaces: ["foo", "bar"],
};
Expand Down Expand Up @@ -122,6 +125,7 @@ it("doesn't call getNamespace when selecting all namespaces", () => {
const setNamespace = jest.fn();
const getNamespace = jest.fn();
const namespace = {
cluster: "defalut",
current: "foo",
namespaces: ["foo", "bar"],
};
Expand Down
41 changes: 11 additions & 30 deletions dashboard/src/components/Header/Header.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import * as React from "react";
// Icons
import { LogOut, Settings } from "react-feather";
import { NavLink } from "react-router-dom";
import "react-select/dist/react-select.css";
import logo from "../../logo.svg";
import { INamespaceState } from "../../reducers/namespace";
import { definedNamespaces } from "../../shared/Namespace";
import { app } from "../../shared/url";
import "./Header.css";
import HeaderLink, { IHeaderLinkProps } from "./HeaderLink";
import HeaderLink from "./HeaderLink";
import NamespaceSelector from "./NamespaceSelector";




interface IHeaderProps {
authenticated: boolean;
fetchNamespaces: () => void;
Expand All @@ -37,26 +34,6 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
featureFlags: { operators: false },
};

// Defines the route
private readonly links: IHeaderLinkProps[] = [
{
children: "Applications",
exact: true,
namespaced: true,
to: "apps",
},
{
children: "Catalog",
namespaced: true,
to: "catalog",
},
{
children: "Service Instances (alpha)",
namespaced: true,
to: "services/instances",
},
];

constructor(props: any) {
super(props);

Expand Down Expand Up @@ -113,11 +90,15 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
<div />
</button>
<ul className="header__nav__menu" role="menubar">
{this.links.map(link => (
<li key={link.to}>
<HeaderLink {...link} currentNamespace={namespace.current} />
</li>
))}
<li>
<HeaderLink to={app.apps.list(namespace.current, namespace.cluster)} exact={true}>Applications</HeaderLink>
</li>
<li>
<HeaderLink to={app.catalog(namespace.current)}>Catalog</HeaderLink>
</li>
<li>
<HeaderLink to={app.servicesInstances(namespace.current)}>Service Instances (alpha)</HeaderLink>
</li>
</ul>
</nav>
)}
Expand Down
24 changes: 4 additions & 20 deletions dashboard/src/components/Header/HeaderLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,20 @@ import { NavLink } from "react-router-dom";
export interface IHeaderLinkProps {
to: string;
exact?: boolean;
external?: boolean;
children?: React.ReactChildren | React.ReactNode | string;
currentNamespace?: string | null;
namespaced?: boolean;
}

class HeaderLink extends React.Component<IHeaderLinkProps> {
public static defaultProps: Partial<IHeaderLinkProps> = {
exact: false,
external: false,
};

public renderInternalLink() {
const { currentNamespace, namespaced, to } = this.props;
const link = currentNamespace && namespaced ? `/ns/${currentNamespace}/${to}` : to;
public render() {
const { to } = this.props;

return (
<NavLink
to={link}
to={to}
activeClassName="header__nav__menu__item-active"
className="header__nav__menu__item"
exact={this.props.exact}
Expand All @@ -31,18 +27,6 @@ class HeaderLink extends React.Component<IHeaderLinkProps> {
</NavLink>
);
}

public renderExternalLink() {
return (
<a href={this.props.to} className="header__nav__menu__item">
{this.props.children}
</a>
);
}

public render() {
return this.props.external ? this.renderExternalLink() : this.renderInternalLink();
}
}

export default HeaderLink;
4 changes: 2 additions & 2 deletions dashboard/src/components/Header/NamespaceSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ it("fetches namespaces and retrive the current namespace", () => {
{...defaultProps}
fetchNamespaces={fetchNamespaces}
getNamespace={getNamespace}
namespace={{ current: "foo", namespaces: [] }}
namespace={{ cluster: "default", current: "foo", namespaces: [] }}
/>,
);
expect(fetchNamespaces).toHaveBeenCalled();
Expand All @@ -103,7 +103,7 @@ it("doesnt' get the current namespace if all namespaces is selected", () => {
{...defaultProps}
fetchNamespaces={fetchNamespaces}
getNamespace={getNamespace}
namespace={{ current: "_all", namespaces: [] }}
namespace={{ cluster: "default", current: "_all", namespaces: [] }}
/>,
);
expect(fetchNamespaces).toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as ReactModal from "react-modal";
import AccessURLTable from "../../containers/AccessURLTableContainer";
import ApplicationStatus from "../../containers/ApplicationStatusContainer";
import itBehavesLike from "../../shared/specs";
import { app } from "../../shared/url";
import AppNotes from "../AppView/AppNotes";
import AppValues from "../AppView/AppValues";
import ResourceTable from "../AppView/ResourceTable";
Expand Down Expand Up @@ -107,7 +108,7 @@ describe("renders a resource", () => {
expect(deleteResource).toHaveBeenCalledWith(defaultProps.namespace, "foo", resource);
// wait async calls
await new Promise(r => r());
expect(push).toHaveBeenCalledWith(`/ns/${defaultProps.namespace}/apps`);
expect(push).toHaveBeenCalledWith(app.apps.list(defaultProps.namespace));
});

it("updates the state with the CRD resources", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class OperatorInstance extends React.Component<IOperatorInstanceProps, IOperator
const deleted = await this.props.deleteResource(namespace, crd!.name.split(".")[0], resource!);
this.closeModal();
if (deleted) {
this.props.push(`/ns/${namespace}/apps`);
this.props.push(app.apps.list(namespace));
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function mapStateToProps(
return {
apps,
filter: qs.parse(location.search, { ignoreQueryPrefix: true }).q || "",
cluster: namespace.cluster,
namespace: namespace.current,
customResources: operators.resources,
isFetchingResources: operators.isFetching,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import UnexpectedErrorPage from "../../components/ErrorAlert/UnexpectedErrorAler
const mockStore = configureMockStore([thunk]);
const makeStore = (error?: { action: string; error: Error }) => {
const state: INamespaceState = {
cluster: "default",
current: "default",
namespaces: ["default"],
error,
Expand Down
15 changes: 9 additions & 6 deletions dashboard/src/containers/RoutesContainer/Routes.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { StaticRouter } from "react-router";
import { Redirect, RouteComponentProps } from "react-router-dom";
import NotFound from "../../components/NotFound";
import RepoListContainer from "../../containers/RepoListContainer";
import { app } from "../../shared/url";
import Routes from "./Routes";


Expand All @@ -27,27 +28,27 @@ const emptyRouteComponentProps: RouteComponentProps<{}> = {
it("invalid path should show a 404 error", () => {
const wrapper = mount(
<StaticRouter location="/random" context={{}}>
<Routes {...emptyRouteComponentProps} namespace={"default"} authenticated={true} />
<Routes {...emptyRouteComponentProps} cluster={"default"} namespace={"default"} authenticated={true} />
</StaticRouter>,
);
expect(wrapper.find(NotFound)).toExist();
expect(wrapper.text()).toContain("The page you are looking for can't be found.");
});

it("should render a redirect to the default namespace", () => {
it("should render a redirect to the default cluster and namespace", () => {
const wrapper = mount(
<StaticRouter location="/" context={{}}>
<Routes {...emptyRouteComponentProps} namespace={"default"} authenticated={true} />
<Routes {...emptyRouteComponentProps} cluster={"default"} namespace={"default"} authenticated={true} />
</StaticRouter>,
);
expect(wrapper.find(NotFound)).not.toExist();
expect(wrapper.find(Redirect).prop("to")).toEqual("/ns/default/apps");
expect(wrapper.find(Redirect).prop("to")).toEqual(app.apps.list("default"));
});

it("should render a redirect to the login page", () => {
const wrapper = mount(
<StaticRouter location="/" context={{}}>
<Routes {...emptyRouteComponentProps} namespace={""} authenticated={true} />
<Routes {...emptyRouteComponentProps} cluster={"default"} namespace={""} authenticated={true} />
</StaticRouter>,
);
expect(wrapper.find(NotFound)).not.toExist();
Expand All @@ -57,14 +58,15 @@ it("should render a redirect to the login page", () => {
it("should render a redirect to the login page (when not authenticated)", () => {
const wrapper = mount(
<StaticRouter location="/" context={{}}>
<Routes {...emptyRouteComponentProps} namespace={"default"} authenticated={false} />
<Routes {...emptyRouteComponentProps} cluster={"default"} namespace={"default"} authenticated={false} />
</StaticRouter>,
);
expect(wrapper.find(NotFound)).not.toExist();
expect(wrapper.find(Redirect).prop("to")).toEqual("/login");
});

describe("Routes depending on feature flags", () => {
const cluster = "default";
const namespace = "default";
const perNamespacePath = "/config/ns/:namespace/repos";

Expand All @@ -73,6 +75,7 @@ describe("Routes depending on feature flags", () => {
<StaticRouter location="/config/repos" context={{}}>
<Routes
{...emptyRouteComponentProps}
cluster={cluster}
namespace={namespace}
authenticated={true}
/>
Expand Down
6 changes: 4 additions & 2 deletions dashboard/src/containers/RoutesContainer/Routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ import ServiceClassListContainer from "../../containers/ServiceClassListContaine
import ServiceClassViewContainer from "../../containers/ServiceClassViewContainer";
import ServiceInstanceListContainer from "../../containers/ServiceInstanceListContainer";
import ServiceInstanceViewContainer from "../../containers/ServiceInstanceViewContainer";
import { app } from "../../shared/url";


type IRouteComponentPropsAndRouteProps = RouteProps & RouteComponentProps<any>;

const privateRoutes = {
"/ns/:namespace/apps": AppListContainer,
"/c/:cluster/ns/:namespace/apps": AppListContainer,
"/ns/:namespace/apps/:releaseName": AppViewContainer,
"/ns/:namespace/apps/:releaseName/upgrade": AppUpgradeContainer,
"/ns/:namespace/apps/new/:repo/:id/versions/:version": AppNewContainer,
Expand All @@ -51,6 +52,7 @@ const routes = {

interface IRoutesProps extends IRouteComponentPropsAndRouteProps {
namespace: string;
cluster: string;
authenticated: boolean;
featureFlags: {
operators: boolean;
Expand Down Expand Up @@ -96,7 +98,7 @@ class Routes extends React.Component<IRoutesProps> {
}
private rootNamespacedRedirect = () => {
if (this.props.namespace && this.props.authenticated) {
return <Redirect to={`/ns/${this.props.namespace}/apps`} />;
return <Redirect to={app.apps.list(this.props.namespace, this.props.cluster)} />;
}
// There is not a default namespace, redirect to login page
return <Redirect to={"/login"} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Routes from "./Routes";

function mapStateToProps({ auth, namespace, config }: IStoreState) {
return {
cluster: namespace.cluster,
namespace: namespace.current || auth.defaultNamespace,
authenticated: auth.authenticated,
featureFlags: config.featureFlags,
Expand Down

0 comments on commit a0fabad

Please sign in to comment.