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
FIX Don’t override ItemRequest if already versioned #60
FIX Don’t override ItemRequest if already versioned #60
Conversation
src/VersionedGridFieldDetailForm.php
Outdated
} | ||
|
||
try { | ||
$obj = new ReflectionClass($class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to use Reflection
to find out if a class is a subclass of another. is_subclass_of
or is_a
will do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sweet Dan! I had thought those only worked on instances of $class
rather than just names. Nice change in 5.3.9
fd9aa21
to
285c61c
Compare
src/VersionedGridFieldDetailForm.php
Outdated
if ($record | ||
&& $record->has_extension(Versioned::class) | ||
&& $record->config()->get('versioned_gridfield_extensions') | ||
) { | ||
$class = VersionedGridFieldItemRequest::class; | ||
// don't override custom classes if they already subclass this | ||
if (!$class) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These conditions can be combined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How'd you mean?
If a developer customises the ItemRequest don’t override it as long as it is already a versioned item hander.
b90f6cc
to
4276951
Compare
@dhensby Will's on leave at the moment - I assume you mean that the if statements in this can be consolidated into one, so I've pushed a fix for that. Would you mind re-checking when you have time please? If this is a bug fix, do you think we could sneak it into 1.0 before a stable release? I hope we can, so have switched the branch to 1.0. |
much better, thanks |
If a developer customises the ItemRequest don’t override it as long as it is already a versioned item hander.