volto icon indicating copy to clipboard operation
volto copied to clipboard

Fix: Show NotFound screen when control panel is not found (#7214)

Open shashank0470 opened this issue 6 months ago โ€ข 13 comments

Overview

This PR addresses Issue #7214, which pointed out that visiting an invalid control panel route (e.g., /controlpanel/does-not-exist) currently results in an empty <div />, leaving users with no feedback.

What Changed

  • Replaced the empty <div /> fallback with the <NotFound /> component inside the Controlpanel component.
  • Now, when an invalid or unknown control panel is accessed, the user is shown the proper "Not Found" page.

Why This Matters

  • Improves user experience by clearly signaling routing errors.
  • Aligns control panel behavior with the rest of Volto's 404 UX.

Preview

When visiting /controlpanel/non-existent-panel, users will now see:

โŒ Not Found

Checklist

  • [x] Fixes the issue behavior
  • [x] Uses existing NotFound component
  • [x] Tested in dev environment
  • [x] Adds clarity to the UX

Thanks to the maintainers for reviewing! ๐Ÿ™Œ

Closes #7214

shashank0470 avatar Jun 26 '25 12:06 shashank0470

@alexandreIFB please review and provide feedback.

stevepiercy avatar Jun 26 '25 20:06 stevepiercy

Thanks for the review @stevepiercy Iโ€™m working on the suggested changes now:

-Reverting changes to package.json

-Updating changelog with credit

-Checking ReadTheDocs failure

Will push updates shortly.

shashank0470 avatar Jun 26 '25 20:06 shashank0470

@shashank0470 please ignore the failing RTD CI checks for now. It's unrelated to this PR, and due to a misconfiguration somewhere that @sneridagh and I need to identify.

stevepiercy avatar Jun 26 '25 21:06 stevepiercy

Thank you @stevepiercy for pointing that out I was already working on those corrections you mentioned. Honestly, the failing ReadTheDocs CI checks were making me feel a bit hopeless, so I really appreciate the clarification that itโ€™s unrelated to this PR.

shashank0470 avatar Jun 26 '25 21:06 shashank0470

All requested changes have been committed. Thanks again @stevepiercy for the helpful feedback! ๐Ÿ™Œ Let me know if anything else needs adjustment.

shashank0470 avatar Jun 26 '25 21:06 shashank0470

I restarted the one failing CI job. Let's see if it fails again.

stevepiercy avatar Jun 26 '25 22:06 stevepiercy

Actually, the implementation isn't as straightforward as it initially appears. Since the component needs to fetch panel data via API, there's a loading period during which showing NotFound based solely on the absence of data would cause users to incorrectly see the 'not found' screen before the request is even completed.

I managed to reach a satisfactory solution, but faced challenges during testing because routes returning 404 weren't triggering error actions in Redux as expected, making it difficult to validate the correct behavior.

/**
 * Controlpanel component.
 * @module components/manage/Controlpanels/Controlpanel
 */

import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { compose } from 'redux';
import { withRouter } from 'react-router-dom';
import Helmet from '@plone/volto/helpers/Helmet/Helmet';
import {
  tryParseJSON,
  extractInvariantErrors,
} from '@plone/volto/helpers/FormValidation/FormValidation';
import { createPortal } from 'react-dom';
import { Button, Container, Dimmer, Loader } from 'semantic-ui-react';
import { defineMessages, FormattedMessage, injectIntl } from 'react-intl';
import { toast } from 'react-toastify';

import Icon from '@plone/volto/components/theme/Icon/Icon';
import Toolbar from '@plone/volto/components/manage/Toolbar/Toolbar';
import Toast from '@plone/volto/components/manage/Toast/Toast';
import { Form } from '@plone/volto/components/manage/Form';
import {
  updateControlpanel,
  getControlpanel,
} from '@plone/volto/actions/controlpanels/controlpanels';

import config from '@plone/volto/registry';

import saveSVG from '@plone/volto/icons/save.svg';
import clearSVG from '@plone/volto/icons/clear.svg';
import NotFound from '@plone/volto/components/theme/NotFound/NotFound';

const messages = defineMessages({
  changesSaved: {
    id: 'Changes saved.',
    defaultMessage: 'Changes saved.',
  },
  back: {
    id: 'Back',
    defaultMessage: 'Back',
  },
  save: {
    id: 'Save',
    defaultMessage: 'Save',
  },
  cancel: {
    id: 'Cancel',
    defaultMessage: 'Cancel',
  },
  info: {
    id: 'Info',
    defaultMessage: 'Info',
  },
  error: {
    id: 'Error',
    defaultMessage: 'Error',
  },
});

/**
 * Controlpanel class.
 * @class Controlpanel
 * @extends Component
 */
class Controlpanel extends Component {
  /**
   * Property types.
   * @property {Object} propTypes Property types.
   * @static
   */
  static propTypes = {
    updateControlpanel: PropTypes.func.isRequired,
    getControlpanel: PropTypes.func.isRequired,
    id: PropTypes.string.isRequired,
    updateRequest: PropTypes.shape({
      loading: PropTypes.bool,
      loaded: PropTypes.bool,
    }).isRequired,
    controlpanel: PropTypes.shape({
      '@id': PropTypes.string,
      data: PropTypes.object,
      schema: PropTypes.object,
      title: PropTypes.string,
    }),
    pathname: PropTypes.string.isRequired,
  };

  /**
   * Default properties.
   * @property {Object} defaultProps Default properties.
   * @static
   */
  static defaultProps = {
    controlpanel: null,
  };

  /**
   * Constructor
   * @method constructor
   * @param {Object} props Component properties
   * @constructs Controlpanel
   */
  constructor(props) {
    super(props);
    this.onCancel = this.onCancel.bind(this);
    this.onSubmit = this.onSubmit.bind(this);
    this.state = { isClient: false, error: null };
  }

  /**
   * Component did mount
   * @method componentDidMount
   * @returns {undefined}
   */
  componentDidMount() {
    this.props.getControlpanel(this.props.id);
    this.setState({ isClient: true });
  }

  /**
   * Component will receive props
   * @method componentWillReceiveProps
   * @param {Object} nextProps Next properties
   * @returns {undefined}
   */
  UNSAFE_componentWillReceiveProps(nextProps) {
    if (this.props.updateRequest.loading && nextProps.updateRequest.error) {
      const message =
        nextProps.updateRequest.error?.response?.body?.error?.message ||
        nextProps.updateRequest.error?.response?.body?.message ||
        nextProps.updateRequest.error?.response?.text ||
        '';

      const error =
        new DOMParser().parseFromString(message, 'text/html')?.all?.[0]
          ?.textContent || message;

      const errorsList = tryParseJSON(error);
      let invariantErrors = [];
      if (Array.isArray(errorsList)) {
        invariantErrors = extractInvariantErrors(errorsList);
      }

      this.setState({ error: error });

      if (invariantErrors.length > 0) {
        toast.error(
          <Toast
            error
            title={this.props.intl.formatMessage(messages.error)}
            content={invariantErrors.join(' - ')}
          />,
        );
      }
    }

    if (this.props.updateRequest.loading && nextProps.updateRequest.loaded) {
      toast.info(
        <Toast
          info
          title={this.props.intl.formatMessage(messages.info)}
          content={this.props.intl.formatMessage(messages.changesSaved)}
        />,
      );
    }
  }

  /**
   * Submit handler
   * @method onSubmit
   * @param {object} data Form data.
   * @returns {undefined}
   */
  onSubmit(data) {
    this.props.updateControlpanel(this.props.controlpanel['@id'], data);
  }

  /**
   * Cancel handler
   * @method onCancel
   * @returns {undefined}
   */
  onCancel() {
    this.props.history.goBack();
  }
  form = React.createRef();

  /**
   * Render method.
   * @method render
   * @returns {string} Markup for the component.
   */
  render() {
    const { filterControlPanelsSchema } = config.settings;
    const { controlpanel, getRequest, listRequest } = this.props;

    if (
      listRequest?.loading ||
      getRequest?.loading ||
      (!getRequest?.loaded && !getRequest?.error)
    ) {
      return (
        <Dimmer active inverted>
          <Loader indeterminate size="small">
            <FormattedMessage id="loading" defaultMessage="Loading" />
          </Loader>
        </Dimmer>
      );
    }

    if (!controlpanel) {
      return <NotFound />;
    }

    return (
      <div id="page-controlpanel">
        <Helmet title={this.props.controlpanel.title} />
        <Container>
          <Form
            ref={this.form}
            title={this.props.controlpanel.title}
            schema={filterControlPanelsSchema(this.props.controlpanel)}
            formData={this.props.controlpanel.data}
            requestError={this.state.error}
            onSubmit={this.onSubmit}
            onCancel={this.onCancel}
            hideActions
            loading={this.props.updateRequest.loading}
          />
        </Container>
        {this.state.isClient &&
          createPortal(
            <Toolbar
              pathname={this.props.pathname}
              hideDefaultViewButtons
              inner={
                <>
                  <Button
                    id="toolbar-save"
                    className="save"
                    aria-label={this.props.intl.formatMessage(messages.save)}
                    onClick={() => this.form.current.onSubmit()}
                    disabled={this.props.updateRequest.loading}
                    loading={this.props.updateRequest.loading}
                  >
                    <Icon
                      name={saveSVG}
                      className="circled"
                      size="30px"
                      title={this.props.intl.formatMessage(messages.save)}
                    />
                  </Button>
                  <Button
                    className="cancel"
                    aria-label={this.props.intl.formatMessage(messages.cancel)}
                    onClick={() => this.onCancel()}
                  >
                    <Icon
                      name={clearSVG}
                      className="circled"
                      size="30px"
                      title={this.props.intl.formatMessage(messages.cancel)}
                    />
                  </Button>
                </>
              }
            />,
            document.getElementById('toolbar'),
          )}
      </div>
    );
  }
}

export default compose(
  injectIntl,
  connect(
    (state, props) => ({
      controlpanel: state.controlpanels.controlpanel,
      getRequest: state.controlpanels.get,
      updateRequest: state.controlpanels.update,
      listRequest: state.controlpanels.list,
      id: props.match.params.id,
      pathname: props.location.pathname,
    }),
    { updateControlpanel, getControlpanel },
  ),
  withRouter,
)(Controlpanel);

alexandreIFB avatar Jun 26 '25 23:06 alexandreIFB

Additionally, I encountered a routing issue where accessing /controlpanel (singular) first routes through /controlpanels (plural), causing Redux state and loading conflicts.

To address this navigation issue, I had to add a check for listRequest?.loading in the component's loading condition. This prevents the NotFound component from being incorrectly displayed during the period when the component is still loading.

alexandreIFB avatar Jun 26 '25 23:06 alexandreIFB

I've implemented all the requested changes. Thanks @stevepiercy and @alexandreIFB for your help and feedback! ๐Ÿ™Œ Please let me know if there's anything else I should do on my side.

shashank0470 avatar Jun 27 '25 07:06 shashank0470

I've implemented all the requested changes. Thanks @stevepiercy and @alexandreIFB for your help and feedback! ๐Ÿ™Œ Please let me know if there's anything else I should do on my side.

Can't find the changes. Did you forget to push?

alexandreIFB avatar Jul 15 '25 17:07 alexandreIFB

@shashank0470 Just a note for next time: itโ€™s important to include a Closes #<issue-number> line in the PR description. Iโ€™ve added it to this one, but having it from the start helps keep our workflow organized, links the PR to the issue automatically, and ensures the issue gets closed when the PR is merged.

wesleybl avatar Nov 28 '25 20:11 wesleybl

Thank you for the reminder! @wesleybl Iโ€™ll make sure to include a Closes # line in the PR description from next time. Appreciate the guidance โ€” this helps me follow the workflow better.

shashank0470 avatar Nov 28 '25 20:11 shashank0470

Many tips and tricks and other guidance for first-timers is in the aptly named First-time contributors. We're always looking into improve it.

stevepiercy avatar Nov 29 '25 02:11 stevepiercy