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.6] Use value() helper in whenLoaded() #24644

Merged
merged 1 commit into from
Jun 21, 2018
Merged

[5.6] Use value() helper in whenLoaded() #24644

merged 1 commit into from
Jun 21, 2018

Conversation

rodrigopedra
Copy link
Contributor

When using a resource it can be useful to pass a Closure as the $default value so it is only evaluated if needed.

Currently the ConditionallyLoadsAttributes attributes already uses value($default) when returning the $default value in the ->when() method, but in the other methods it was missing.

My use case:

// in a Resource
    public function toArray( $request )
    {
        return [
            'id'          => $this->resource->id,
            'name'        => $this->resource->name,
            'description' => $this->resource->description,
            'users_count' => $this->whenLoaded('users',
                function () {return $this->resource->users->count();},
                function () {return $this->resource->users()->count();} // not possible today
            )
        ];
    }

@rodrigopedra rodrigopedra changed the title Use value() when using default values [5.6] Use value() when using default values Jun 20, 2018
@@ -188,7 +188,7 @@ protected function whenPivotLoaded($table, $value, $default = null)
protected function transform($value, callable $callback, $default = null)
{
return transform(
$value, $callback, func_num_args() === 3 ? $default : new MissingValue
$value, $callback, func_num_args() === 3 ? value($default) : new MissingValue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transform() global helper function already calls the Closure in that $default argument:

        if (is_callable($default)) {
            return $default($value);
        }
>>> transform('', function () { return 'foo'; }, function () { return 'bar'; })
=> "bar"

@rodrigopedra
Copy link
Contributor Author

Thanks! Pushed a new commit just adding value() helper in the whenLoaded() method

@rodrigopedra rodrigopedra changed the title [5.6] Use value() when using default values [5.6] Use value() helper in whenLoaded() Jun 21, 2018
@sisve
Copy link
Contributor

sisve commented Jun 21, 2018

Can you add a test that verifies this new behavior? It will mostly be used to avoid regressions in case someone accidentally removes this code change in the future.

@rodrigopedra
Copy link
Contributor Author

Hi @sisve , thanks for the suggestion, I wrote some tests to both then when() and whenLoaded() methods to test against default values. If you can review it I'll appreciate.

@taylorotwell taylorotwell merged commit 2c02bf7 into laravel:5.6 Jun 21, 2018
@rodrigopedra rodrigopedra deleted the resource-default-value branch June 21, 2018 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants