-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[10.x] Fixes singleton and api singletons creatable|destryoable|only|except combinations #47098
Conversation
…oyable|only|except options
e08f004
to
5594b54
Compare
Hey @nunomaduro In your implementation, @timacdonald and I noticed an inconsistency between We've rolled back your change to the We no longer need any singleton-specific code in the We've made the change as a single commit so you can roll back if you don't agree :) |
@@ -427,15 +427,14 @@ public function apiSingletons(array $singletons, array $options = []) | |||
*/ | |||
public function apiSingleton($name, $controller, array $options = []) | |||
{ | |||
$only = ['show', 'update', 'destroy']; | |||
$only = ['store', 'show', 'update', 'destroy']; |
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.
@jessarcher why was store
added here?
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.
We previously had some singleton-specific overrides in getResourceMethods
that were applied after only
, leading to unexpected behavior.
Now, the creatable
and destroyable
methods just expand the defaults that are passed to getResourceMethods
, which can then be reduced by only
.
We need to specify store
here so that it will be included when using creatable
. It's safe to include even when creatable
is not called because only
can only reduce the defaults, but not expand them.
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.
Ready for review @taylorotwell.
This pull request fixes #47088, and the usage of
creatable
ordestroyable
Singleton Resources when combined with options such asonly
orexcept
.@jessarcher Can you let know what do you think about this, before I deliver it to Taylor? Thanks!