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

Value Metrics Count $groupBy no longer allows DB::Raw expression #6365

Closed
cparkinsonMYCM opened this issue Apr 26, 2024 · 4 comments
Closed
Labels
bug Verified bug by the Nova team fix incoming A fix is in review
Milestone

Comments

@cparkinsonMYCM
Copy link

  • Laravel Version: 10.48.9
  • Nova Version: 4.33.3
  • PHP Version: 8.1

Description:

We have a couple of metrics whose calculate method looks like this:

return $this->count($request, Model::withoutGlobalScopes(), DB::Raw("ISNULL(`some_id`)"))
	->label(fn($value) => $value ? "A" : "B")
	->colors([
		"A" => "#66FF66",
		"B" => "#6666FF",
	]);

These used to work but at some point it seems the formatAggregateResult method has changed to explode the $groupBy value assuming it's a string and then directly accessing the $groupBy column, so now we can't group by the calculated field.

@crynobone crynobone added the needs more info More information is required label Apr 26, 2024
@crynobone
Copy link
Member

Unable to reproduce the issue, please provide full reproducing repository based on fresh installation as suggested in the bug report template (or you can refer to https://github.com/nova-issues for example)

@cparkinsonMYCM
Copy link
Author

provide full reproducing repository based on fresh installation

I don't have enough time to go through that setup, so we'll just work around it instead...
Feel free to close.

But for context, if it matters.

Passing a DB::Raw('...') expresssion as the $groupby gives this error:

local.ERROR: explode(): Argument #2 ($string) must be of type string, Illuminate\Database\Query\Expression given {"userId":"1eef1014-ebe2-6d6a-a70e-7912c197fbd9","exception":"[object] (TypeError(code: 0): explode(): Argument #2 ($string) must be of type string, Illuminate\\Database\\Query\\Expression given at /var/www/html/vendor/laravel/nova/src/Metrics/Partition.php:126)

Here:

/**
     * Format the aggregate result for the partition.
     *
     * @param  \Illuminate\Database\Eloquent\Model  $result
     * @param  string  $groupBy
     * @return array<string|int, int|float>
     */
    protected function formatAggregateResult($result, $groupBy)
    {
        $key = with($result->{last(explode('.', $groupBy))}, function ($key) {
            return Util::value($key);
        });

        if (! is_int($key)) {
            $key = (string) $key;
        }

        return [$key => $result->aggregate ?? 0];
    }

@crynobone
Copy link
Member

The doc block parameter of $groupBy is and has always been string. Illuminate\Database\Query\Expression however no longer has __toString() method since Laravel 10 which would cause this issue.

@crynobone crynobone added this to the 5.x milestone May 6, 2024
@crynobone crynobone removed the needs more info More information is required label May 6, 2024
@crynobone crynobone modified the milestones: 5.x, 4.x May 14, 2024
@crynobone crynobone added the bug Verified bug by the Nova team label May 14, 2024
@crynobone crynobone added the fix incoming A fix is in review label May 21, 2024
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Verified bug by the Nova team fix incoming A fix is in review
Projects
None yet
Development

No branches or pull requests

2 participants