Skip to content
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.

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

[Remove discover] Migrate AngularJS routing to React #6680

Closed
19 tasks done
Tracked by #6120
asteriscos opened this issue May 16, 2024 · 22 comments · Fixed by #6689
Closed
19 tasks done
Tracked by #6120

[Remove discover] Migrate AngularJS routing to React #6680

asteriscos opened this issue May 16, 2024 · 22 comments · Fixed by #6689
Assignees
Labels
level/task Task issue type/enhancement Enhancement issue

Comments

@asteriscos
Copy link
Member

asteriscos commented May 16, 2024

Description

In the last stage of removing the Discover legacy we must refactor the AngularJS routing system into React routing. This will allow us to fully deprecated AngularJS.

This effort involves several tasks and has many breaking changes. So we will work with a feature branch and finally merge it in the 4.9.0 branch once it's done to ensure we don't merge any half-done work and break the app.

Tasks

  • Replace the routing based on AngularJS to ReactJS

    • Analyze and port route resolvers to ReactJS
      • Port route resolvers
    • Replace the navigation
      • Create navigation service
      • Create unit test for the navigation service
      • Adapt the navigation between views
      • Create custom routing based on search parameters
        • Components
          • Switch
          • Route
          • Redirect
        • hooks
        • HOCs
  • Remove dependencies related to AngularJS

  • Remove unused code

    • Remove withRexduxProvider HOC
    • Remove WzReduxProvider component
  • Adapt tests

  • Nice to have:

These tasks could be completed, partially completed or postponed to another iteration
Some of the following topics appeared as suggestions:

  • Replace the management of view to display based on component state in favor to URL (routing). This enables to the user to share the URL to other users, open in another browser tab or refresh the page and see the same view. Moreover simplify the components that are managing the view based on component states by using the routing. For example, the application uses some search parameters to open specific views that are no the main of the route, but only uses the search parameter when accessing from specific UI elements, they are no used by the main view.
  • Enhance the URLs: currently the application uses search parameters to display different views and this could be using path name instead. Example: from /settings?tab=api to /settings/api
  • Remove the # from the URL path

Design

Development considerations:

  • Keep as possible the current URLs and navigation based on path name and search params
  • As in the routing based on AngularJS after the creation on multiple applications, the routes of each application should be similar to the commented routing.

This means the applications have routes that display data realted to another application. As in the previous routing, we should avoid the application displays routes that are related to another applicaiton, in this case, we should change to the expected application instead. Ideally, the routing of the application should be specific to the application, but this is considered out of the scope of this issue and could be done in another iteration.

The application should mount a ReactJS application into the HTML element provided by the parameters received on the mount method of the Wazuh dashboard application. The application could be wrapped in the root level with some contexts as I18nProvider and Redux provider to these are accesibles from any component in the components tree:

export async function renderApp(params) {
  /* Load or initiate the dependencies, async styles, etc... */
  ReactDOM.render(
    <I18nProvider>
      <Provider store={store}>
        <Application {...deps} />
      </Provider>
    </I18nProvider>,
    params.element,
  );
  return () => ReactDOM.unmountComponentAtNode(params.element);

For routing, we create a history that is managed through the navigation service. This approach lets push new URL changes and access to the current location for services that are not decoupled from the components. The navigation service is the way to manage the navigation instead of using the interface provided by the react-router-dom library. It stores a references to the history we create.

The Application component defines the layout of the application under the router provided by react-router-dom. In this step, we port the current routing based on AngularJS to ReactJS.

// app-router
const Application =  () => {
  // general states or effects

  return (
    <Router history={history}>
     {/* General components that could be always displayed in all views or conditionally */}
     <Switch>
      <Route path={'/health-check'} exact render={HealthCheck}></Route>
      {/* Rest of routes */}
     <Switch>  
    <Router>
  )
}

Some views are managed depending on the search parameters ( tab=syscollector), and the current version of react-router-dom that is v5 is unable to re-render components when the search parameters change. To cover this required navigation based on search paremeter, we use a custom routing components/hooks/HOCs that have a similar interface to the provided by the react-router-dom.

@asteriscos asteriscos changed the title Migrate AngularJS routing to React [Remove discover] Migrate AngularJS routing to React May 16, 2024
@asteriscos asteriscos added level/task Task issue type/enhancement Enhancement issue labels May 16, 2024
@Desvelao Desvelao self-assigned this May 17, 2024
@jbiset
Copy link
Member

jbiset commented May 17, 2024

Update 16/05/2024

The documentation available in the code is analyzed in the Example Routing_Example (Example/Routing_Example), however, the PATH with the Routing Development Documentation presented by the Readme no longer exists:

@jbiset
Copy link
Member

jbiset commented May 17, 2024

Research 17/05/2024

The possibility of using Routes instead of Switch was analyzed. However, OSD uses the "react-router-dom" version: "^5.3.0" and it was only from V6 that Switch was replaced by Routes. Based on this, we have to work considering V5 of react-router-dom.
In the plugins/main/public/plugin.ts file, any reference to Angular was removed, which apparently no longer influenced the behavior of this component.
The problem of duplication of the URL base when using menus was analyzed. It may be that the URLs defined are not correct with the changes for the React migration and cause them to be concatenated. This leads to the idea of ​​generating a url-builder that defines and is responsible for creating the URLs in a certain and correct way, just as OSD does in url-builder.tsx. Reusing it could also be analyzed.

@Desvelao
Copy link
Member

Development branch: feat/6680-migrate-app-routing

@jbiset
Copy link
Member

jbiset commented May 20, 2024

Update 20/05/2024

  • Removed Angular from the PinnedAgentManager component.
  • Attaches agent selector modal to ButtonPinnedAgent
  • The mechanism to use to update the URL and whether or not it is necessary to reload the page when updating it is analyzed.
  • For now URLSearchParams is used but it is necessary to carefully analyze whether this method or one of OSD's own methods should be used.

@gdiazlo gdiazlo linked a pull request May 21, 2024 that will close this issue
6 tasks
@Desvelao
Copy link
Member

Desvelao commented May 21, 2024

Update 2024/05/21

  • Create HOC to simulate the route resolvers logic from AngularJS
  • Port the route resolvers
  • Fix a problem related to circular dependency
  • Adapt the main components of the routes
  • Fix the redirection from /health-check to the previous location

@jbiset
Copy link
Member

jbiset commented May 21, 2024

Update 21/05/2024

The mount property of the object receiving core.applicaiton.register is a function that has params of type AppMountParameters as a parameter. When you use this parameter in renderApp you are already implicitly passing a history part of params. Analyzing the src/core/public/application/types.ts file, there is also an example that shows how to incorporate that history into the app's router. With which we begin to test using that history and creating a createGetterSetter in kibana-services to be able to directly access the history, as the PinnedAgentManager would need, as well as other components, whether class or functional.

AppMountParameters interface
/** @public */
export interface AppMountParameters<HistoryLocationState = unknown> {
  /**
   * The container element to render the application into.
   */
  element: HTMLElement;

  /**
   * A scoped history instance for your application. Should be used to wire up
   * your applications Router.
   *
   * @example
   * How to configure react-router with a base path:
   *
   * ```ts
   * // inside your plugin's setup function
   * export class MyPlugin implements Plugin {
   *   setup({ application }) {
   *     application.register({
   *      id: 'my-app',
   *      appRoute: '/my-app',
   *      async mount(params) {
   *        const { renderApp } = await import('./application');
   *        return renderApp(params);
   *      },
   *    });
   *  }
   * }
   * ```
   *
   * ```ts
   * // application.tsx
   * import React from 'react';
   * import ReactDOM from 'react-dom';
   * import { Router, Route } from 'react-router-dom';
   *
   * import { CoreStart, AppMountParameters } from 'src/core/public';
   * import { MyPluginDepsStart } from './plugin';
   *
   * export renderApp = ({ element, history }: AppMountParameters) => {
   *   ReactDOM.render(
   *     <Router history={history}>
   *       <Route path="/" exact component={HomePage} />
   *     </Router>,
   *     element
   *   );
   *
   *   return () => ReactDOM.unmountComponentAtNode(element);
   * }
   * ```
   */
  history: ScopedHistory<HistoryLocationState>;

  /**
   * The route path for configuring navigation to the application.
   * This string should not include the base path from HTTP.
   *
   * @deprecated Use {@link AppMountParameters.history} instead.
   *
   * @example
   *
   * How to configure react-router with a base path:
   *
   * ```ts
   * // inside your plugin's setup function
   * export class MyPlugin implements Plugin {
   *   setup({ application }) {
   *     application.register({
   *      id: 'my-app',
   *      appRoute: '/my-app',
   *      async mount(params) {
   *        const { renderApp } = await import('./application');
   *        return renderApp(params);
   *      },
   *    });
   *  }
   * }
   * ```
   *
   * ```ts
   * // application.tsx
   * import React from 'react';
   * import ReactDOM from 'react-dom';
   * import { BrowserRouter, Route } from 'react-router-dom';
   *
   * import { CoreStart, AppMountParameters } from 'src/core/public';
   * import { MyPluginDepsStart } from './plugin';
   *
   * export renderApp = ({ appBasePath, element }: AppMountParameters) => {
   *   ReactDOM.render(
   *     // pass `appBasePath` to `basename`
   *     <BrowserRouter basename={appBasePath}>
   *       <Route path="/" exact component={HomePage} />
   *     </BrowserRouter>,
   *     element
   *   );
   *
   *   return () => ReactDOM.unmountComponentAtNode(element);
   * }
   * ```
   */
  appBasePath: string;

  /**
   * A function that can be used to register a handler that will be called
   * when the user is leaving the current application, allowing to
   * prompt a confirmation message before actually changing the page.
   *
   * This will be called either when the user goes to another application, or when
   * trying to close the tab or manually changing the url.
   *
   * @example
   *
   * ```ts
   * // application.tsx
   * import React from 'react';
   * import ReactDOM from 'react-dom';
   * import { BrowserRouter, Route } from 'react-router-dom';
   *
   * import { CoreStart, AppMountParameters } from 'src/core/public';
   * import { MyPluginDepsStart } from './plugin';
   *
   * export renderApp = ({ element, history, onAppLeave }: AppMountParameters) => {
   *    const { renderApp, hasUnsavedChanges } = await import('./application');
   *    onAppLeave(actions => {
   *      if(hasUnsavedChanges()) {
   *        return actions.confirm('Some changes were not saved. Are you sure you want to leave?');
   *      }
   *      return actions.default();
   *    });
   *    return renderApp({ element, history });
   * }
   * ```
   */
  onAppLeave: (handler: AppLeaveHandler) => void;

  /**
   * A function that can be used to set the mount point used to populate the application action container
   * in the chrome header.
   *
   * Calling the handler multiple time will erase the current content of the action menu with the mount from the latest call.
   * Calling the handler with `undefined` will unmount the current mount point.
   * Calling the handler after the application has been unmounted will have no effect.
   *
   * @example
   *
   * ```ts
   * // application.tsx
   * import React from 'react';
   * import ReactDOM from 'react-dom';
   * import { BrowserRouter, Route } from 'react-router-dom';
   *
   * import { CoreStart, AppMountParameters } from 'src/core/public';
   * import { MyPluginDepsStart } from './plugin';
   *
   * export renderApp = ({ element, history, setHeaderActionMenu }: AppMountParameters) => {
   *    const { renderApp } = await import('./application');
   *    const { renderActionMenu } = await import('./action_menu');
   *    setHeaderActionMenu((element) => {
   *      return renderActionMenu(element);
   *    })
   *    return renderApp({ element, history });
   * }
   * ```
   */
  setHeaderActionMenu: (menuMount: MountPoint | undefined) => void;
  /**
   * Optional datasource id to pass while mounting app
   */
  dataSourceId?: string;
}
ScopedHistory class
/**
 * A wrapper around a `History` instance that is scoped to a particular base path of the history stack. Behaves
 * similarly to the `basename` option except that this wrapper hides any history stack entries from outside the scope
 * of this base path.
 *
 * This wrapper also allows Core and Plugins to share a single underlying global `History` instance without exposing
 * the history of other applications.
 *
 * The {@link ScopedHistory.createSubHistory | createSubHistory} method is particularly useful for applications that
 * contain any number of "sub-apps" which should not have access to the main application's history or basePath.
 *
 * @public
 */
export class ScopedHistory<HistoryLocationState = unknown>
  implements History<HistoryLocationState> {
  /**
   * Tracks whether or not the user has left this history's scope. All methods throw errors if called after scope has
   * been left.
   */
  private isActive = true;
  /**
   * All active listeners on this history instance.
   */
  private listeners = new Set<LocationListener<HistoryLocationState>>();
  /**
   * Array of the local history stack. Only stores {@link Location.key} to use tracking an index of the current
   * position of the window in the history stack.
   */
  private locationKeys: Array<string | undefined> = [];
  /**
   * The key of the current position of the window in the history stack.
   */
  private currentLocationKeyIndex: number = 0;

  constructor(private readonly parentHistory: History, private readonly basePath: string) {
    const parentPath = this.parentHistory.location.pathname;
    if (!parentPath.startsWith(basePath)) {
      throw new Error(
        `Browser location [${parentPath}] is not currently in expected basePath [${basePath}]`
      );
    }

    this.locationKeys.push(this.parentHistory.location.key);
    this.setupHistoryListener();
  }

  /**
   * Creates a `ScopedHistory` for a subpath of this `ScopedHistory`. Useful for applications that may have sub-apps
   * that do not need access to the containing application's history.
   *
   * @param basePath the URL path scope for the sub history
   */
  public createSubHistory = <SubHistoryLocationState = unknown>(
    basePath: string
  ): ScopedHistory<SubHistoryLocationState> => {
    return new ScopedHistory<SubHistoryLocationState>(this, basePath);
  };

  /**
   * The number of entries in the history stack, including all entries forwards and backwards from the current location.
   */
  public get length() {
    this.verifyActive();
    return this.locationKeys.length;
  }

  /**
   * The current location of the history stack.
   */
  public get location() {
    this.verifyActive();
    return this.stripBasePath(this.parentHistory.location as Location<HistoryLocationState>);
  }

  /**
   * The last action dispatched on the history stack.
   */
  public get action() {
    this.verifyActive();
    return this.parentHistory.action;
  }

  /**
   * Pushes a new location onto the history stack. If there are forward entries in the stack, they will be removed.
   *
   * @param pathOrLocation a string or location descriptor
   * @param state
   */
  public push = (
    pathOrLocation: Path | LocationDescriptorObject<HistoryLocationState>,
    state?: HistoryLocationState
  ): void => {
    this.verifyActive();
    if (typeof pathOrLocation === 'string') {
      this.parentHistory.push(this.prependBasePath(pathOrLocation), state);
    } else {
      this.parentHistory.push(this.prependBasePath(pathOrLocation));
    }
  };

  /**
   * Replaces the current location in the history stack. Does not remove forward or backward entries.
   *
   * @param pathOrLocation a string or location descriptor
   * @param state
   */
  public replace = (
    pathOrLocation: Path | LocationDescriptorObject<HistoryLocationState>,
    state?: HistoryLocationState
  ): void => {
    this.verifyActive();
    if (typeof pathOrLocation === 'string') {
      this.parentHistory.replace(this.prependBasePath(pathOrLocation), state);
    } else {
      this.parentHistory.replace(this.prependBasePath(pathOrLocation));
    }
  };

  /**
   * Send the user forward or backwards in the history stack.
   *
   * @param n number of positions in the stack to go. Negative numbers indicate number of entries backward, positive
   *          numbers for forwards. If passed 0, the current location will be reloaded. If `n` exceeds the number of
   *          entries available, this is a no-op.
   */
  public go = (n: number): void => {
    this.verifyActive();
    if (n === 0) {
      this.parentHistory.go(n);
    } else if (n < 0) {
      if (this.currentLocationKeyIndex + 1 + n >= 1) {
        this.parentHistory.go(n);
      }
    } else if (n <= this.currentLocationKeyIndex + this.locationKeys.length - 1) {
      this.parentHistory.go(n);
    }
    // no-op if no conditions above are met
  };

  /**
   * Send the user one location back in the history stack. Equivalent to calling
   * {@link ScopedHistory.go | ScopedHistory.go(-1)}. If no more entries are available backwards, this is a no-op.
   */
  public goBack = () => {
    this.verifyActive();
    this.go(-1);
  };

  /**
   * Send the user one location forward in the history stack. Equivalent to calling
   * {@link ScopedHistory.go | ScopedHistory.go(1)}. If no more entries are available forwards, this is a no-op.
   */
  public goForward = () => {
    this.verifyActive();
    this.go(1);
  };

  /**
   * Not supported. Use {@link AppMountParameters.onAppLeave}.
   *
   * @remarks
   * We prefer that applications use the `onAppLeave` API because it supports a more graceful experience that prefers
   * a modal when possible, falling back to a confirm dialog box in the beforeunload case.
   */
  public block = (
    prompt?: boolean | string | TransitionPromptHook<HistoryLocationState>
  ): UnregisterCallback => {
    throw new Error(
      `history.block is not supported. Please use the AppMountParameters.onAppLeave API.`
    );
  };

  /**
   * Adds a listener for location updates.
   *
   * @param listener a function that receives location updates.
   * @returns an function to unsubscribe the listener.
   */
  public listen = (
    listener: (location: Location<HistoryLocationState>, action: Action) => void
  ): UnregisterCallback => {
    this.verifyActive();
    this.listeners.add(listener);
    return () => {
      this.listeners.delete(listener);
    };
  };

  /**
   * Creates an href (string) to the location.
   * If `prependBasePath` is true (default), it will prepend the location's path with the scoped history basePath.
   *
   * @param location
   * @param prependBasePath
   */
  public createHref = (
    location: LocationDescriptorObject<HistoryLocationState>,
    { prependBasePath = true }: { prependBasePath?: boolean } = {}
  ): Href => {
    this.verifyActive();
    if (prependBasePath) {
      location = this.prependBasePath(location);
      if (location.pathname === undefined) {
        // we always want to create an url relative to the basePath
        // so if pathname is not present, we use the history's basePath as default
        // we are doing that here because `prependBasePath` should not
        // alter pathname for other method calls
        location.pathname = this.basePath;
      }
    }
    return this.parentHistory.createHref(location);
  };

  private prependBasePath(path: Path): Path;
  private prependBasePath(
    location: LocationDescriptorObject<HistoryLocationState>
  ): LocationDescriptorObject<HistoryLocationState>;
  /**
   * Prepends the scoped base path to the Path or Location
   */
  private prependBasePath(
    pathOrLocation: Path | LocationDescriptorObject<HistoryLocationState>
  ): Path | LocationDescriptorObject<HistoryLocationState> {
    if (typeof pathOrLocation === 'string') {
      return this.prependBasePathToString(pathOrLocation);
    } else {
      return {
        ...pathOrLocation,
        pathname:
          pathOrLocation.pathname !== undefined
            ? this.prependBasePathToString(pathOrLocation.pathname)
            : undefined,
      };
    }
  }

  /**
   * Prepends the base path to string.
   */
  private prependBasePathToString(path: string): string {
    return path.length ? `${this.basePath}/${path}`.replace(/\/{2,}/g, '/') : this.basePath;
  }

  /**
   * Removes the base path from a location.
   */
  private stripBasePath(location: Location<HistoryLocationState>): Location<HistoryLocationState> {
    return {
      ...location,
      pathname: location.pathname.replace(new RegExp(`^${this.basePath}`), ''),
    };
  }

  /** Called on each public method to ensure that we have not fallen out of scope yet. */
  private verifyActive() {
    if (!this.isActive) {
      throw new Error(
        `ScopedHistory instance has fell out of navigation scope for basePath: ${this.basePath}`
      );
    }
  }

  /**
   * Sets up the listener on the parent history instance used to follow navigation updates and track our internal
   * state. Also forwards events to child listeners with the base path stripped from the location.
   */
  private setupHistoryListener() {
    const unlisten = this.parentHistory.listen((location, action) => {
      // If the user navigates outside the scope of this basePath, tear it down.
      if (!location.pathname.startsWith(this.basePath)) {
        unlisten();
        this.isActive = false;
        return;
      }

      /**
       * Track location keys using the same algorithm the browser uses internally.
       * - On PUSH, remove all items that came after the current location and append the new location.
       * - On POP, set the current location, but do not change the entries.
       * - On REPLACE, override the location for the current index with the new location.
       */
      if (action === 'PUSH') {
        this.locationKeys = [
          ...this.locationKeys.slice(0, this.currentLocationKeyIndex + 1),
          location.key,
        ];
        this.currentLocationKeyIndex = this.locationKeys.indexOf(location.key); // should always be the last index
      } else if (action === 'POP') {
        this.currentLocationKeyIndex = this.locationKeys.indexOf(location.key);
      } else if (action === 'REPLACE') {
        this.locationKeys[this.currentLocationKeyIndex] = location.key;
      } else {
        throw new Error(`Unrecognized history action: ${action}`);
      }

      [...this.listeners].forEach((listener) => {
        listener(this.stripBasePath(location as Location<HistoryLocationState>), action);
      });
    });
  }
}

Likewise, in the same file (src/core/public/application/types.ts) there are two methods (navigateToApp and navigateToUrl) that can be useful when determining navigation management, either through applications and directly to an absolute URL.

@Desvelao
Copy link
Member

Update 2024/05/22

  • Minor code cleaning
  • Adapt the deletion of the API token when logout
  • Add menu and wzNotReadyYet components
  • Fix redirection from Endpoints summary agents table to agent view
  • Remove usage of AngularJS injector in some services and components
  • Remove deprecated components and services related to AngularJS

@jbiset
Copy link
Member

jbiset commented May 22, 2024

Update 22/05/2024

Taking into account the needs to manage navigation in the app with respect to history and taking into account the possibility of using the OSD history mechanism. It is decided to generate a NavigationService object that centralizes and abstracts the navigation of the app. Different strategies were tried, but it was decided to use a Singleton which allows specifying the history to be used, giving flexibility to the use of one's own history or that of the OSD.

NavigationService diagram

classDiagram
    class NavigationService {
        - static instance: NavigationService
        - history: History
        + static getInstance(history?: History): NavigationService
        - constructor(history: History)
        + navigate(path: string, state?: any): void
        + replace(path: string, state?: any): void
        + goBack(): void
        + goForward(): void
        + go(n: number): void
        + getLocation(): Location
        + listen(listener: (location: Location, action: Action) => void): void
    }

    class History {
        + push(path: string, state?: any): void
        + replace(path: string, state?: any): void
        + goBack(): void
        + goForward(): void
        + go(n: number): void
        + location: Location
        + listen(listener: (location: Location, action: Action) => void): void
    }

    NavigationService --> History : uses

@Desvelao
Copy link
Member

Update 2024/05/23

  • Cleaning source code removing deprecated/unused components

deprecated view and services of Vulnerabilities > Inventory
Redux action creators and reducers related to the deprecated system to render the dashboards
Components:
SecurityAlerts
ModulesHelper
KibanaVis
ClusterTimelions
KibanaVisWrapper
Metrics
WzVisualize
WzFilterBar
Dashboard
HOCs:
withModuleTabLoader
hooks:
useRefreshAngularDiscover
useRootScope
AngularJS services:
DiscoverPendingUpdates
LoadedVisualizations
RawVisualizations
VisHandlers
VisFactoryHandler

  • Remove usage of AngularJS on main component related to modules
  • Fix some redirections

@Desvelao
Copy link
Member

Update 2024/05/24

  • Remove AngularJS view templates
  • Research about redirections and view management
  • Create basic components to route based on search query parameters
  • Adapt the routing based on state of components to the search parameters

@Desvelao
Copy link
Member

Desvelao commented May 24, 2024

Route based on search URL parameters

The current URLs of the application uses a combination of paths and search parameters of the URL to display a view.

For example, the /manager path has multiple views that are controlled by the tab search parameters.

The router provided by react-router-dom v5 does not re-render the components under the same path. In this scenarion and to keep the same URLs of the current application, I created some basic components similar to the router provided by react-router-dom package that routes using the search query parameters. These new components let to keep the same URLs.

Example the implementation of the router based on the search parameters replacing the routing based on the state of the component:

import { Redirect, Route, Switch } from '../../router-search';
// ...
export const ViewRouter = () => {
  // Bussiness logic of component
  return (
    <Switch>
        <Route path='?tab=syscollector'>
          <MainModuleAgent agent={agent} section={tab} />
          <MainSyscollector agent={agent} />
        </Route>
        <Route path='?tab=stats'>
          <MainModuleAgent agent={agent} section={tab} />
          <MainAgentStats agent={agent} />
        </Route>
        <Route path='?tab=configuration'>
          <MainModuleAgent agent={agent} section={tab} />
          <MainAgentStats agent={agent} />
        </Route>
        <Route path='?tab=welcome'>
          <AgentsWelcome
            switchTab={switchTab}
            agent={agent}
            pinAgent={pinnedAgentManager.pinAgent}
            unPinAgent={pinnedAgentManager.unPinAgent}
          />
        </Route>
        <Redirect to='?tab=welcome'></Redirect>
      </Switch>
  )
}

As these components have a similar interface and names to the provided by react-router-dom, we could replace the routing based on search parameters to using the path instead of replacing the import of the components and adapting the path and to props of Route and Redirect components.

Moreover, a new hook was created to detect the changes on the search parameters, called useRouterSearch.

Example:

const CustomComponent = () => {
  // The search contains all the search parameters as an object
  const search = useRouterSearch();

  return // render components
}

Take into account into the routing of the application could change in the development phase. We are porting the current routing keeping the similar URLs to the current applications.

@Desvelao
Copy link
Member

Desvelao commented May 27, 2024

Update 2024/05/27

  • Remove unused code and files related to AngularJS

Research about the unused files and methods

  • Replace usage of window.location by the router interfaces
  • Fix some redirections
  • Replace the references to panels as value of the tabView search parameter by dashboard (before this change, it used both values to display the Dashboard tab of the modules)
  • Remove withReduxProvider HOC and usage
  • Remove WzReduxProvider component and usage

@jbiset
Copy link
Member

jbiset commented May 28, 2024

Update 28/05/2024

  • Agent selection problem in the agent selection modal is corrected.
  • The history that comes from params in the mount to render the application apparently does not communicate with react router, this means that when doing a push or replace on history the components are not updated according to the routes. Because of this, the history used by NavigationService is changed to a history created with createHashHistory at the beginning of the app.
  • New methods are added to NavigationService to obtain different parameters about the URL and history and the types of the navigate and replace parameters are improved.
  • Change useRouterSearch to use NavigationService to get location data.
  • Changed Redirect in plugins/main/public/components/router-search/index.ts to use NavigationService for navigation.

TODO:
Analyze and implement, if possible, a general mechanism so that changes to the query params of the URL are taken into account by the router, without the need to put a hook for this operation in each place where it is needed.

@jbiset
Copy link
Member

jbiset commented May 29, 2024

Update 29/05/2024

  • Replaced everywhere useHistory was used with NavigationService
  • HashRouter is changed to Router to be able to use a custom history based on createHashHistory

Changed_router_import

  • Replace NavigationService with whatever is appropriate in the useEffect of plugins/main/public/components/overview/mitre/intelligence/resource.tsx
  • Methods are added to NavigationService and existing ones are improved.
classDiagram
    class NavigationService {
        - static instance: NavigationService
        - history: History
        + static getInstance(history?: History): NavigationService
        + getHistory(): History
        + getLocation(): Location
        + getHash(): string
        + getPathname(): string
        + getSearch(): string
        + getState(): any
        + getParams(): URLSearchParams
        + renewURL(params?: URLSearchParams): void
        + navigate(path: string, state?: any): void
        + replace(path: string, state?: any): void
        + goBack(): void
        + goForward(): void
        + go(n: number): void
        + reload(): void
        + listen(listener: (location: Location, action: Action) => void): () => void
    }

    class HashHistory {
        + push(path: string, state?: any): void
        + replace(path: string, state?: any): void
        + goBack(): void
        + goForward(): void
        + go(n: number): void
        + location: Location
        + listen(listener: (location: Location, action: Action) => void): void
    }

    NavigationService --> HashHistory : uses

TODO:

  • Fix this behavior so that details are not lost when removing the query params and adding the path to the history on plugins/main/public/components/overview/mitre/intelligence/resource.tsx.
  • See windows.location.reload replacement
  • See AppNavigate regarding NavigateService
  • See replacement navigateTo with respect to NavigateService
  • See replacement navigateToApp with respect to NavigateService
  • Analyze and implement reload solution when changing API

@Desvelao
Copy link
Member

Desvelao commented May 30, 2024

Update selected API or alerts index pattern

The current application reloads the page when changing the selected API or index pattern using the selectors of the header. It should not be necessary if the application was designed to react to these changes and reloading the required data with the new selection.

We decided to research about the possibility to avoid the page is refreshed when changing the selected API or index pattern, but it is not possible with out adapting the component to react to this changes. This is out of the scope of this issue, so we could manage it in another issue if we consider.

So, I replaced the reload of AngularJS by a refresh of the page based on window.location.reload through a method exposed by the NavigationService ( NavigationService.reload() ) that was recently added.

@jbiset
Copy link
Member

jbiset commented May 30, 2024

Update 30/05/2024

  • Fixed behavior so that details are not lost when removing the query params and adding the path to the history on plugins/main/public/components/overview/mitre/intelligence/resource.tsx.
  • All files to be modified that have a method related to navigateTo... are analyzed and listed.

TODO:

  • See windows.location.reload replacement
  • See AppNavigate regarding NavigateService
    • See replacement navigateToApp with respect to NavigateService
    • See replacement navigateToModule with respect to NavigateService
    • See replacement navigateToUrl with respect to NavigateService
  • Analyze getUrlForApp method of getCore().application. (used for example in plugins/main/public/components/common/welcome/components/fim_events_table/fim_events_table.tsx)

navigateToModule

  • plugins/main/public/components/agents/fim/inventory/fileDetail.tsx
  • plugins/main/public/components/common/modules/discover/discover.tsx
  • plugins/main/public/components/common/wazuh-discover/render-columns.tsx
  • plugins/main/public/components/common/welcome/components/mitre_top/mitre-top.tsx
  • plugins/main/public/components/overview/mitre/framework/components/techniques/components/flyout-technique/flyout-technique-columns.tsx
  • plugins/main/public/components/overview/mitre/framework/components/techniques/components/flyout-technique/flyout-technique.tsx
  • plugins/main/public/controllers/management/components/management/statistics/prompt-statistics-disabled.tsx
  • plugins/main/public/react-services/app-navigate.js

navigateToApp

  • plugins/main/public/components/common/globalBreadcrumb/platformBreadcrumb.tsx
  • plugins/main/public/components/common/util/agent-group-truncate/group-truncate.tsx
  • plugins/main/public/components/common/welcome/components/requirement_vis/requirement_vis.tsx
  • plugins/main/public/components/common/welcome/components/sca_scan/sca_scan.tsx
  • plugins/main/public/components/endpoints-summary/table/agents-table.test.tsx
  • plugins/main/public/components/endpoints-summary/table/actions/actions.tsx
  • plugins/main/public/components/management/cluster/dashboard/dashboard.tsx
  • plugins/main/public/components/overview/compliance-table/components/requirement-flyout/requirement-flyout.tsx
  • plugins/main/public/components/overview/vulnerabilities/common/hocs/validate-vulnerabilities-states-index-pattern.tsx
  • plugins/main/public/components/wz-blank-screen/wz-blank-screen.js
  • plugins/main/public/controllers/management/components/management/groups/group-agents-table.js
  • plugins/main/public/controllers/overview/components/stats.js
  • plugins/main/public/controllers/overview/components/stats.test.tsx
  • plugins/main/test/functional/page_objects/wazuh_common_page.js
  • plugins/wazuh-check-updates/public/components/updates-notification.test.tsx

navigateToUrl

  • plugins/main/public/components/health-check/container/health-check.container.test.tsx
  • plugins/main/public/components/visualize/components/sample-data-warning.test.js

@jbiset
Copy link
Member

jbiset commented May 31, 2024

Update 31/05/2024

Since navigateToApp, navigateToUrl and getUrlForApp are methods provided directly by the OSD core (see CoreStart and ApplicationStart interfaces) and work correctly with the new Wazuh routing configuration without Angular, it was decided to maintain their use. However, to centralize the use of navigation and URL management, it was decided to create the same methods in the NavigationService, so that if in the future there is a need to change its implementation, only the corresponding methods will have to be modified in NavigationService.

ApplicationStart interface: `src/core/public/application/types.ts`
export interface ApplicationStart {
  /**
   * Gets the read-only capabilities.
   */
  capabilities: RecursiveReadonly<Capabilities>;

  /**
   * Observable emitting the list of currently registered apps and their associated status.
   *
   * @remarks
   * Applications disabled by {@link Capabilities} will not be present in the map. Applications manually disabled from
   * the client-side using an {@link AppUpdater | application updater} are present, with their status properly set as `inaccessible`.
   */
  applications$: Observable<ReadonlyMap<string, PublicAppInfo>>;

  /**
   * Navigate to a given app
   *
   * @param appId
   * @param options - navigation options
   */
  navigateToApp(appId: string, options?: NavigateToAppOptions): Promise<void>;

  /**
   * Navigate to given URL in a SPA friendly way when possible (when the URL will redirect to a valid application
   * within the current basePath).
   *
   * The method resolves pathnames the same way browsers do when resolving a `<a href>` value. The provided `url` can be:
   * - an absolute URL
   * - an absolute path
   * - a path relative to the current URL (window.location.href)
   *
   * If all these criteria are true for the given URL:
   * - (only for absolute URLs) The origin of the URL matches the origin of the browser's current location
   * - The resolved pathname of the provided URL/path starts with the current basePath (eg. /mybasepath/s/my-space)
   * - The pathname segment after the basePath matches any known application route (eg. /app/<id>/ or any application's `appRoute` configuration)
   *
   * Then a SPA navigation will be performed using `navigateToApp` using the corresponding application and path.
   * Otherwise, fallback to a full page reload to navigate to the url using `window.location.assign`
   *
   * @example
   * ```ts
   * // current url: `https://opensearch-dashboards:8080/base-path/s/my-space/app/dashboard`
   *
   * // will call `application.navigateToApp('discover', { path: '/some-path?foo=bar'})`
   * application.navigateToUrl('https://opensearch-dashboards:8080/base-path/s/my-space/app/discover/some-path?foo=bar')
   * application.navigateToUrl('/base-path/s/my-space/app/discover/some-path?foo=bar')
   * application.navigateToUrl('./discover/some-path?foo=bar')
   *
   * // will perform a full page reload using `window.location.assign`
   * application.navigateToUrl('https://elsewhere:8080/base-path/s/my-space/app/discover/some-path') // origin does not match
   * application.navigateToUrl('/app/discover/some-path') // does not include the current basePath
   * application.navigateToUrl('/base-path/s/my-space/app/unknown-app/some-path') // unknown application
   * application.navigateToUrl('../discover') // resolve to `/base-path/s/my-space/discover` which is not a path of a known app.
   * application.navigateToUrl('../../other-space/discover') // resolve to `/base-path/s/other-space/discover` which is not within the current basePath.
   * ```
   *
   * @param url - an absolute URL, an absolute path or a relative path, to navigate to.
   */
  navigateToUrl(url: string): Promise<void>;

  /**
   * Returns the absolute path (or URL) to a given app, including the global base path.
   *
   * By default, it returns the absolute path of the application (e.g `/basePath/app/my-app`).
   * Use the `absolute` option to generate an absolute url instead (e.g `http://host:port/basePath/app/my-app`)
   *
   * Note that when generating absolute urls, the origin (protocol, host and port) are determined from the browser's current location.
   *
   * @param appId
   * @param options.path - optional path inside application to deep link to
   * @param options.absolute - if true, will returns an absolute url instead of a relative one
   */
  getUrlForApp(appId: string, options?: { path?: string; absolute?: boolean }): string;

  /**
   * Register a context provider for application mounting. Will only be available to applications that depend on the
   * plugin that registered this context. Deprecated, use {@link CoreSetup.getStartServices}.
   *
   * @deprecated
   * @param contextName - The key of {@link AppMountContext} this provider's return value should be attached to.
   * @param provider - A {@link IContextProvider} function
   */
  registerMountContext<T extends keyof AppMountContext>(
    contextName: T,
    provider: IContextProvider<AppMountDeprecated, T>
  ): void;

  /**
   * An observable that emits the current application id and each subsequent id update.
   */
  currentAppId$: Observable<string | undefined>;
}
CoreStart interface: `src/core/public/index.ts`
export interface CoreStart {
  /** {@link ApplicationStart} */
  application: ApplicationStart;
  /** {@link ChromeStart} */
  chrome: ChromeStart;
  /** {@link DocLinksStart} */
  docLinks: DocLinksStart;
  /** {@link HttpStart} */
  http: HttpStart;
  /** {@link SavedObjectsStart} */
  savedObjects: SavedObjectsStart;
  /** {@link I18nStart} */
  i18n: I18nStart;
  /** {@link NotificationsStart} */
  notifications: NotificationsStart;
  /** {@link OverlayStart} */
  overlays: OverlayStart;
  /** {@link IUiSettingsClient} */
  uiSettings: IUiSettingsClient;
  /** {@link FatalErrorsStart} */
  fatalErrors: FatalErrorsStart;
  /**
   * exposed temporarily until https://github.com/elastic/kibana/issues/41990 done
   * use *only* to retrieve config values. There is no way to set injected values
   * in the new platform.
   * @deprecated
   * */
  injectedMetadata: {
    getInjectedVar: (name: string, defaultValue?: any) => unknown;
    getBranding: () => Branding;
  };
  /** {@link WorkspacesStart} */
  workspaces: WorkspacesStart;
}
classDiagram

    
    class CoreStart {
        ApplicationStart application
        ChromeStart chrome
        DocLinksStart docLinks
        HttpStart http
        SavedObjectsStart savedObjects
        I18nStart i18n
        NotificationsStart notifications
        OverlayStart overlays
        IUiSettingsClient uiSettings
        FatalErrorsStart fatalErrors
        InjectedMetadata injectedMetadata
        WorkspacesStart workspaces
    }

    class ApplicationStart {
        +RecursiveReadonly capabilities
        +Observable applications$
        +Promise navigateToApp(appId: string, options?: NavigateToAppOptions)
        +Promise navigateToUrl(url: string)
        +string getUrlForApp(appId: string, options?: unknown)
        +void registerMountContext
        +Observable currentAppId$
    }

    CoreStart --> ApplicationStart

navigateToModule is a method that does not come from the OSD core. However, analyzing the different places that it is used and given that it has conditionalities according to the parameters that are passed to it, among them according to the type of event, it must be analyzed if it is necessary to generate a corresponding method in NavigationService, or if it should be modified the implementation of navigateToModule so that it uses some NavigationService method, or if it is not necessary to change it.

navigateToModule static method of AppNavigate: `plugins/main/public/react-services/app-navigate.js`
static navigateToModule(e, section, params, navigateMethod = false) {
    e.persist(); // needed to access this event asynchronously
    if (e.button == 0) {
      // left button clicked
      if (navigateMethod) {
        navigateMethod();
        return;
      }
    }
    getIndexPattern().then((indexPattern) => {
      const urlParams = {};

      if (Object.keys(params).length) {
        Object.keys(params).forEach((key) => {
          if (key === 'filters') {
            urlParams['_w'] = this.buildFilter_w(params[key], indexPattern);
          } else {
            urlParams[key] = params[key];
          }
        });
      }
      const url = Object.entries(urlParams)
        .map((e) => e.join('='))
        .join('&');
      const currentUrl = window.location.href.split('#/')[0];
      const newUrl = currentUrl + `#/${section}?` + url;

      if (e && (e.which == 2 || e.button == 1)) {
        // middlebutton clicked
        window.open(newUrl, '_blank', 'noreferrer');
      } else if (e.button == 0) {
        // left button clicked
        if (navigateMethod) {
          navigateMethod();
        } else {
          window.location.href = newUrl;
        }
      }
    });
  }

Based on the above, the navigateToApp and getUrlForApp methods were replaced in the following list. The navigateToUrl method did not need to be replaced anywhere because in the files in which it is used they were functions mocked in tests.

navigateToApp

  • plugins/main/public/components/common/globalBreadcrumb/platformBreadcrumb.tsx
  • plugins/main/public/components/common/util/agent-group-truncate/group-truncate.tsx
  • plugins/main/public/components/common/welcome/components/requirement_vis/requirement_vis.tsx
  • plugins/main/public/components/common/welcome/components/sca_scan/sca_scan.tsx
  • plugins/main/public/components/endpoints-summary/table/agents-table.test.tsx
  • plugins/main/public/components/endpoints-summary/table/actions/actions.tsx
  • plugins/main/public/components/management/cluster/dashboard/dashboard.tsx
  • plugins/main/public/components/overview/compliance-table/components/requirement-flyout/requirement-flyout.tsx
  • plugins/main/public/components/overview/vulnerabilities/common/hocs/validate-vulnerabilities-states-index-pattern.tsx
  • plugins/main/public/components/wz-blank-screen/wz-blank-screen.js
  • plugins/main/public/controllers/management/components/management/groups/group-agents-table.js
  • plugins/main/public/controllers/overview/components/stats.js
  • plugins/main/public/controllers/overview/components/stats.test.tsx
  • plugins/main/test/functional/page_objects/wazuh_common_page.js
  • plugins/wazuh-check-updates/public/components/updates-notification.test.tsx

getUrlForApp

  • plugins/main/public/components/agents/stats/agent-stats.tsx
  • plugins/main/public/components/agents/syscollector/main.tsx
  • plugins/main/public/components/common/modules/main-agent.tsx
  • plugins/main/public/components/common/welcome/agents-welcome.js
  • plugins/main/public/components/common/welcome/overview-welcome.js
  • plugins/main/public/components/common/welcome/components/menu-agent.js
  • plugins/main/public/components/common/welcome/components/fim_events_table/fim_events_table.tsx
  • plugins/main/public/components/endpoints-summary/index.tsx
  • plugins/main/public/components/endpoints-summary/register-agent/containers/register-agent/register-agent.tsx
  • plugins/main/public/components/endpoints-summary/table/agents-table.tsx
  • plugins/main/public/components/health-check/container/health-check.container.tsx
  • plugins/main/public/components/visualize/components/sample-data-warning.js
  • plugins/main/public/controllers/management/components/management/configuration/configuration-main.js
  • plugins/main/public/controllers/management/components/management/configuration/configuration-switch.js
  • plugins/main/public/controllers/management/components/management/ruleset/views/rule-info.tsx
  • plugins/main/public/controllers/management/components/management/statistics/statistics-overview.js
  • plugins/main/public/react-services/reporting.js

navigateToUrl

  • plugins/main/public/components/health-check/container/health-check.container.test.tsx
  • plugins/main/public/components/visualize/components/sample-data-warning.test.js

NOTE:
plugins/wazuh-check-updates/public/components/updates-notification.tsx uses the getCore().application.getUrlForApp method, but it is not replaced by NavigationService since it belongs to another plugin. It remains to analyze what to do in these cases.

TODO

  • Perform cross-testing of the functionalities associated with the files changed in the list above

@jbiset
Copy link
Member

jbiset commented Jun 3, 2024

Update 03/06/2024

  • Migrate AppNavigate methods to NavigationService
    • Migrate navigateToModule and the buildFilter_w function to NavigationService. TODO is left to analyze its improvement in the future.
    • The static getUrlParameter method of AppNavigate is not migrated because it is not necessary, for NavigationService it has that functionality.
    • Replace used methods of AppNavigate in the App
      • Replaced AppNavigate.getUrlParameter functionality in plugins/main/public/components/settings/configuration/components/header.tsx
      • Replaced AppNavigate.navigateToModule in the following files:
        • plugins/main/public/components/agents/fim/inventory/fileDetail.tsx
        • plugins/main/public/components/common/modules/discover/discover.tsx
        • plugins/main/public/components/common/wazuh-discover/render-columns.tsx
        • plugins/main/public/components/common/welcome/components/mitre_top/mitre-top.tsx
        • plugins/main/public/components/overview/mitre/framework/components/techniques/components/flyout-technique/flyout-technique-columns.tsx
        • plugins/main/public/components/overview/mitre/framework/components/techniques/components/flyout-technique/flyout-technique.tsx
        • plugins/main/public/controllers/management/components/management/statistics/prompt-statistics-disabled.tsx
      • The /plugins/main/public/react-services/app-navigate.js file is removed
  • Improve navigateToModule in NavigationService
    • Replaced use of window.location.href in navigateToModule.
  • Develop NavigationService test
    • Progress is being made with the mock and some tests for NavigationService

TO DO:

  • Fix bug when use link group on configuration Agent preview

@Desvelao
Copy link
Member

Desvelao commented Jun 4, 2024

Update 2024/06/04

  • Fix unit tests
  • Replace usage of routing with the interface of react-router-dom in favor to NavigatioNService
  • Remove unused code

@Desvelao
Copy link
Member

Desvelao commented Jun 4, 2024

Problem

When using the link group on configuration on Agent > :agent > Configuration causes an error in the view seems

Research

The bug seems to be related due the AgentView component is unmounted (due to changing of the application), but this is mounted again. It seems the problem is caused because the applications are sharing the same history and when navigating to another Wazuh application, the initial location does not the expected search parameters causing an error on the view.

Ensuring each Wazuh application has a different history solves the problem.

I am not sure because all the applications are sharing the same history managed by NavigationService. We will discuss it in the sync meeting today.

@jbiset
Copy link
Member

jbiset commented Jun 4, 2024

Update 04/06/2024

  • The NavigationService test has been implemented.
  • The app's consistency is tested with respect to routing changes in search of corner cases.
  • Several tests are added to NavigationService, however there are still some methods to add tests that had problems when mocking getCore. Also, the goBack() and go() methods are not affecting history. For now the latter is not blocking since it is not used, but it would be desirable for it to work correctly and with its test. (TO DO)

@jbiset
Copy link
Member

jbiset commented Jun 5, 2024

Update 05/06/2024

  • A general guide is added to perform the review of PR 6689
  • Added tests for navigateToApp, navigateToUrl, getUrlForApp, goBack, goForward and go(n) methods of NavigationService.

The particular problem with testing NavigationService's goBack, goForward, and go(n) methods is that this is an asynchronous operation, meaning that navigation back/forward/n does not complete immediately. To resolve this, we leverage the listen method to listen for changes in history and ensure that the test waits for the navigation to complete before making expectations.
In order to complete these tests, it was useful to review the tests of the OSD routing itself present in the following files:

  • src/core/public/application/integration_tests/router.test.tsx
  • src/core/public/application/scoped_history.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
level/task Task issue type/enhancement Enhancement issue
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants