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

onAdminSave($event) passes page as $event['object'] or $event['page'] depending on regular/flex page #2995

Closed
pamtbaau opened this issue Aug 23, 2020 · 6 comments
Assignees

Comments

@pamtbaau
Copy link
Contributor

When subscribing to onAdminSave event, a different Event object is passed when saving a 'regular' page versus a 'flex' page.

regular: $event['object']
flex: $event['page']

This will break plugins that want to intercept the saving of a page.

@mahagr
Copy link
Member

mahagr commented Aug 25, 2020

For Flex Pages it should be the same, yes (which looks like to be a bug), but the event passes object variable in the older version, too, so you need to check if you're saving a page or not.

@pamtbaau
Copy link
Contributor Author

pamtbaau commented Aug 25, 2020

Yes, the type of object passed into eventhandler is being checked.

Current workaround:

$page = $event['object'] ?? $event['page'];

if (!$page instanceof Page && !$page instanceof PageObject) {
    return;
}

@rhukster
Copy link
Member

The bug is that they should both send the page as ['page'] on the event, however now it’s sent as PageInterface and that can be used the same for regular pages or flex. Should not be ['object']

@mahagr mahagr transferred this issue from getgrav/grav-plugin-admin Aug 28, 2020
@mahagr mahagr self-assigned this Aug 28, 2020
mahagr added a commit that referenced this issue Aug 28, 2020
@mahagr
Copy link
Member

mahagr commented Aug 28, 2020

@rhukster Looks like there are a lot of inconsistencies in how the admin deals with the page events. All the other events use page instead of object.

@mahagr
Copy link
Member

mahagr commented Aug 28, 2020

I updated the other events to use object as well (page variable is still there for backwards compatibility).

@mahagr mahagr added the fixed label Aug 28, 2020
@pamtbaau
Copy link
Contributor Author

Thanks @mahagr 👍

@mahagr mahagr closed this as completed Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants