64robots/nova-fields

JSON field stores empty data if last child field is empty

Closed this issue · 4 comments

Hi all,

Don't know exactly since which version of Laravel Nova, but at least at v3.11.1 with laravel 7.26.1, the JSON field does not store any values if the last child field is empty. As far as I've been able to test, it does not matter which child elements it has.

To Recreate the bug:

in Nova/Resource/MyModelRerource.php:

class MyModelResource extends Resource
{
    public function fields(): array
    {
        return [
             //...
            JSON::make('Info', [
                Text::make('Function'),
                Select::make('Gender')
                    ->options([null => '-', 'male' => 'Male', 'female' => 'Female']),
                Checkboxes::make('Diet') // from package Silvanite\NovaFieldCheckboxes\Checkboxes
                     ->options(['vegan' => 'Vegan', 'vegetarian' => 'Vegetarian', 'gluten_free' => 'Gluten Free']),
                Boolean::make('Newsletter'),
                Text::make('Company')
            ])
                ->flatten(),
             //...
        ];
    }
}

Filled as follows:
Screenshot 2020-11-03 at 15 53 51

on update, following request is sent to backend:
Screenshot 2020-11-03 at 15 55 16

and after saving fields look like this:
Screenshot 2020-11-03 at 16 02 34

In the database, these fields now also are empty

ok, after some more investigation I don't really know if the bug is with this package, or with Laravel itself or with Nova:

The problem seems to be the way the attributes are being converted in this package while storing. In short it looks something like this:

$this->model->{ $attribute . '->' . $field->attribute} = $value;

When value is null, instead of setting that sub attribute as null, it stores the whole attribute as null.

I can reproduce it here in tinker as well:

>>> $i = ReplyLive\Models\Invitee::find(8037)
=> ReplyLive\Models\Invitee {#4736
     id: 8037,
     info: ""eyJpdiI6InNPZ0IxUlFVMFBtekJMZzNlUjdjUUE9PSIsInZhbHVlIjoiUDd0KzB6MFZmR1JsSW0zY3RsZ0hWOTkxY25obUNsdVVqRHNJU2R3RjVubThpLzJoUUhZNTRpUWxPV1RrU3JwRTcyMmtOQ3NqSzhpNmYwRkk2cVVuREVQTWE4Q0M4SGtqNXgwSzFiLzZhYWV2ZVM3UVhzcmFrbTRPWkdaTkordEVTalBFOFFjdXRjV3lCMVdEVmpxZFFqYnBuaVBoMHQvckhnVkpkNWpibm9nODBXckVEUG0vYWNUdXB6a1FVbVh5ZUVMcSt5aFUrbVRCZmtyWjcwTldiZz09IiwibWFjIjoiZjZmZmQwZGEwNzFiMjZkMDJkMGYyNWExNzE0NGMxMjY4ZjZhZTBjYjQ3NTJjM2RlYTQ0MGE1NjNjZTNjMjY1ZSJ9"",
   }
>>> $i->info
=> [
     "function" => "aa",
     "gender" => "female",
     "diet" => [
       "vegetarian",
     ],
     "newsletter" => "yes",
     "company" => "ffasd",
   ]
>>> $i->{"info->newsletter"} = "no"
=> "no"
>>> $i->info
=> [
     "function" => "aa",
     "gender" => "female",
     "diet" => [
       "vegetarian",
     ],
     "newsletter" => "no",
     "company" => "ffasd",
   ]
>>> $i->{"info->newsletter"} = null
=> null
>>> $i->info
=> null

Also weird: as you can see I've the info field encrypted. If I take the encryption off of this field, the arrow notation for setting the sub attribute does not work at all:

>>> $i = ReplyLive\Models\Invitee::withTrashed()->find(8037)
=> ReplyLive\Models\Invitee {#4734
     id: 8037,
     info: "{"value":"something"}",
   }
>>> $i->info
=> [
     "value" => "something",
   ]
>>> $i->{"info->value"} = "something else"
=> "something else"
>>> $i->info
=> "something else"

Field is casted as array by the way. casting as json seems to give the same result

Ok, 1 night of sleep later and I've found something in our Encryptable trait we use on our models, which breaks this.

<?php

namespace ReplyLive\Traits;

use Illuminate\Support\Str;

trait Encryptable
{
    public function attributesToArray()
    {
        $attributes = parent::attributesToArray();

        foreach ($this->getEncryptableFields() as $key) {
            if (array_key_exists($key, $attributes) && isset($attributes[$key])) {
                $attributes[$key] = decrypt($attributes[$key]);
            }
        }

        return $attributes;
    }

    public function getAttribute($key)
    {
        $value = parent::getAttribute($key);

        if (in_array($key, $this->getEncryptableFields()) && isset($value)) {
            try {
                $value = decrypt($value);
            } catch (\Illuminate\Contracts\Encryption\DecryptException $e) {
                return $value;
            }
        }
        return $value;
    }

    public function setAttribute($key, $value)
    {
        if (Str::contains($key, '->')) {
            [$key, $path] = explode('->', $key, 2);
        }

        if (in_array($key, $this->getEncryptableFields()) && isset($value)) {
            if (isset($path)) {
                $value = array_merge(
                    $this->{$key} ?? [],
                    ["$path" => $value]
                );
            }

            $value = encrypt($value);
        }

        return parent::setAttribute($key, $value);
    }

    public function getEncryptableFields(): array
    {
        return property_exists($this, 'encryptable') ? $this->encryptable : [];
    }
}

On line 42 the check isset($value) returns false when $value === null, therefore encryption is not being used and the whole field is set to null.

This is more a blog on fixing my own stuff now than having an actual bug in this package, but I'll keep you up to date with my progress. Also, still doesn't explain why code also breaks when I've had encryption disabled for the given field.....

Thank you so much for all your time. We will keep an eye on this issue and see if we can help with anything

Hi Javier,

First off, I needed it fixes, so no better way then deep diving into it myself ;-). I've been able to fix it with our encryptable field by changing the setAttribute() function a bit:

function setAttribute($key, $value)
    {
        if (Str::contains($key, '->')) {
            [$key, $path] = explode('->', $key, 2);
        }

        if (!in_array($key, $this->getEncryptableFields())) {
            return parent::setAttribute($key, $value);
        }

        if (isset($path)) {
            $value = array_merge(
                $this->{$key} ?? [],
                ["$path" => $value]
            );
        }

        $value = isset($value) ? encrypt($value) : null;

        return parent::setAttribute($key, $value);
    }

This way nested values will be set correctly, while still be able to have literal null on encrypted field instead of an encrypted version of null. Only limit our trait now has for array values is "deep nesting", The explode limit of 2 will prevent setting a value like $user->info['extra']['data'] with the arrow notation $user->{"info->extra->data"} = 'something. For our usecase not a problem, but for others might be an issue.

Anyway, I'll see if I find the time to completely test setting nested values without our encryptable trait, since I had some weird behaviour with that as well.

If I don't find anything, I'll close this issue, otherwise I'll keep you all up to date!