Skip to content

ActionEvent unable to capture changes from non-scalar attribute values #3980

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

Closed
Synchro opened this issue Apr 13, 2022 · 9 comments
Closed
Labels
pending Issues that are pending triage
Milestone

Comments

@Synchro
Copy link

Synchro commented Apr 13, 2022

  • Laravel Version: 8.82
  • Nova Version: 3.32.0
  • PHP Version: 8.0.17
  • Database Driver & Version: MySQL 8.0.17
  • Operating System and Version: n/a
  • Browser type and version: n/a
  • Reproduction Repository: https://github.com/###/###

Description:

I have a resource that contains a field which is a cast object in its own right (actually a Point type, which contains two float properties for lat & long) that are mapped into two Number fields in Nova. When updating, the object is converted to a binary string format by its cast so that it's ready to write to the DB, but this string is exposed in Nova and can't be encoded as JSON (because it's not valid UTF-8), so the update fails with this error:

Unable to encode attribute [original] for model [Laravel\Nova\Actions\ActionEvent] to JSON: Malformed UTF-8 characters, possibly incorrectly encoded.

I don't know why it's exposed here, but it is.

Two Nova resource fields point at this single underlying object's properties, with fillUsing and resolveUsing to set and get them:

                Number::make('Latitude', 'latitude')
                    ->fillUsing(function ($request, $model, $attribute, $requestAttribute) {
                        $model->location->latitude = (float)$request->input($requestAttribute);
                    })
                    ->resolveUsing(function ($value, \App\Models\Location $model) {
                        return $model->location->latitude;
                    }),
                Number::make('Longitude', 'longitude')
                    ->fillUsing(function ($request, $model, $attribute, $requestAttribute) {
                        $model->location->longitude = (float)$request->input($requestAttribute);
                    })
                    ->resolveUsing(function ($value, \App\Models\Location $model) {
                        return $model->location->longitude;
                    }),

ActionEvent::forResourceUpdate makes the resource, but I note that the original property mentioned in the error is created like this:

        'original' => array_intersect_key($model->getRawOriginal(), $model->getDirty()),

getRawOriginal bypasses casts and exposes the raw binary string, so I'm assuming this is where the problem lies.

Ultimately, it's not clear to me whether this is a Nova issue (that it shouldn't be exposing the uncast value) or a problem in the Point class (that it's not casting in the right place).

I apologise that this is not a clearer bug report because I don't know how it's meant to work, so I can't tell what's deviating from what's expected.

Detailed steps to reproduce the issue on a fresh Nova installation:

@crynobone crynobone added the pending Issues that are pending triage label Apr 13, 2022
@Synchro
Copy link
Author

Synchro commented Apr 13, 2022

I have created a minimal repo that demonstrates this issue. It's obviously private, but I've added the package author and @crynobone.

@crynobone crynobone added this to the 3.x milestone Apr 14, 2022
@crynobone crynobone changed the title Updating resources with casts exposes uncast values ActionEvent unable to capture changes from non-scalar attribute values Apr 20, 2022
@Synchro
Copy link
Author

Synchro commented Apr 25, 2022

There is another but possibly related problem with this same class that appears in Nova. A not-null point field in MySQL must have a spatial index, but this also means that it must have a default value in the model. Given that the property is not scalar, a default value can't be set in the property itself. Setting it in a constructor can't work because prior to casting, the property does not exist, and populating a default overwrites the real value since it seems that Nova (or possibly Laravel itself) only populates the values later. Nova seems to create a model instance even when there are no instances in the DB, and the net result is that the index view in Nova for this same resource reports a can't get <property> on null error. This is visible in the demo project I created.

@goaround
Copy link

@Synchro I run into the same issue with Nova 4 and https://github.com/MatanYadaev/laravel-eloquent-spatial Did you found a workaround?

@Synchro
Copy link
Author

Synchro commented Apr 27, 2022

No, I'm still stuck. The maintainer of that package thinks it's a Nova issue as it generally works outside Nova. From what I've seen, I suspect that's probably correct.

@goaround
Copy link

Ok, thanks. I moved back to grimzy/laravel-mysql-spatial and used the PR from Shift: grimzy/laravel-mysql-spatial#184

@pavloniym
Copy link

pavloniym commented May 23, 2022

Same problem. My workaround:

class GeoobjectGeometry extends AbstractEloquentModel {

    // Casts
    protected $casts = [
        'center' => Point::class,
        'coordinates' => Geometry::class,
    ];
    
    // .....

    public function getRawOriginal($key = null, $default = null)
    {
        foreach (['center', 'coordinates'] as $cast) {
            $this->original[$cast] = Geometry::fromWkb(Arr::get($this->original, $cast, $default));
        }

        return Arr::get($this->original, $key, $default);
    }

}

@Synchro
Copy link
Author

Synchro commented May 26, 2022

@pavloniym While I can see why that would work, it strikes me as a bit hacky, fixing the symptom rather than the cause (which I still don't understand). Is it not likely to cause issues if something that's expecting to have to decode raw values obtained from $original gets a value that's already decoded?

I can see problems if that function was called twice, since the changes are written back to $original. Perhaps something non-destructive and more selective?:

    public function getRawOriginal($key = null, $default = null)
    {
        if (in_array($key, ['center', 'coordinates'])) {
            return Geometry::fromWkb(Arr::get($this->original, $key, $default));
        } 
        return Arr::get($this->original, $key, $default);
    }

Rather than having to put those property names into an array like that it would be nice to get that info from the $casts array, though I'm not sure how you would check that Point::class is a subclass of Geometry::class. It might be that I'm trying to optimise an ugly hack here...

@pavloniym
Copy link

@Synchro Good point about twice call. Your workaround is better in this case, nice job! 👍

I have many other "casted" fields in this model (they are skipped from my code example above). And just for simplicity, I listed only specific (Geometry) fields in array.
And you right — I classify this as hack too =) But I couldn't find any other such simple workarounds.

@crynobone
Copy link
Member

In Nova 4, we no longer store changes from non-scalar attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending Issues that are pending triage
Projects
None yet
Development

No branches or pull requests

4 participants