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

[5.x]: Setting isValid to false in Structures::EVENT_BEFORE_MOVE_ELEMENT causes Server Error #15292

Closed
rogerdawkins opened this issue Jul 3, 2024 · 5 comments
Labels

Comments

@rogerdawkins
Copy link

What happened?

Description

I would like to control Moves and Inserts in a Structure so users cannot add new Level 1 elements but can add and move elements at Level 2. New entries will be added to Level 2 with a specific parent.

I've generated a simple module with the Craft generator which works fine as a bare module. I have then added a Structures::EVENT_BEFORE_MOVE_ELEMENT event and as a very simple test I have set $event->isValid to false. This works and stops any moves in the Structure. However, at that point it also displays 'A server error occurred.' message. I also tried adding $event->handled = true but that didn't help.

I'm running locally with DDEV and have restarted that. I've looked in the logs but no reference to the server error.

Steps to reproduce

  1. Generate empty module using the Craft Generator - craft make module
  2. The new module includes an attachEventHandlers method. Inside that I added:
Event::on(
    Structures::class,
    Structures::EVENT_BEFORE_MOVE_ELEMENT,
    function (MoveElementEvent $event) {
        // stop the move happening
        $event->isValid = false;        
    }
);
  1. Move an element in a structure

Expected behavior

Stop the Move happening without a Server Error message.

Actual behavior

Stops the move but displays the Server Error message.

Craft CMS version

5.2.5

PHP version

8.2.20

Operating system and version

Linux 6.5.0-42-generic

Database type and version

MySQL 8.0.36

Image driver and version

Imagick 3.7.0 (ImageMagick 6.9.11-60)

Installed plugins and versions

  • CKEditor 4.x-dev
  • Control Panel CSS v3.x-dev
  • Feed Me 6.x-dev
  • PDF Transform 5.0.1
  • Sprig dev-develop
  • Vite v5.x-dev
@brandonkelly
Copy link
Member

The “server error” message is a bit misleading. All the JS code knows is that the server didn’t accept the new location, so it shows that message and reloads the elements so they revert to their actual positions. That could be due to an actual error that gets logged, but in your case it’s just the result of you setting $event->isValid = false.

@rogerdawkins
Copy link
Author

@brandonkelly OK, that's good to know that I'm not doing anything wrong. Can I replace the 'server error' message with something more meaningful for my editors like 'Move not allowed'. I tried adding a setNotice but it doesn't display until the page reloads again.

@rogerdawkins
Copy link
Author

@brandonkelly Thinking about this some more. When you try to drag an entry to a illegal position, such as level 3 of a structure with a max of 2 levels, the system will block it silently, with no Server Error message. So that's what I'm trying to achieve. Effectively setting isValid to false I think should replicate an attempted illegal move. Do you think that could be possible?

@brandonkelly
Copy link
Member

Just made a change for Craft 4.11 and 5.3 that makes it possible to provide a custom message, by throwing an exception rather than setting $event->isValid = false.

use yii\base\UserException;

Event::on(
    Structures::class,
    Structures::EVENT_BEFORE_MOVE_ELEMENT,
    function (MoveElementEvent $event) {
        // stop the move happening
        throw new UserException('This move isn’t allowed.');      
    }
);

Note that the exception must be an instance/subclass of yii\base\UserException for the message to be shown when Craft isn’t running in Dev Mode.

Thinking about this some more. When you try to drag an entry to a illegal position, such as level 3 of a structure with a max of 2 levels, the system will block it silently, with no Server Error message. So that's what I'm trying to achieve.

In that case, the drop target isn’t available to begin with. So there’s no actual move operation that was ever attempted, as far as the back end knows.

@rogerdawkins
Copy link
Author

@brandonkelly Nice one! That makes it much more friendly, trying to tell users to ignore a 'server error' was going to be a hard sell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants