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

Doesn't work with multiple select. Array to string conversion exception #31

Open
terion-name opened this issue Apr 27, 2017 · 14 comments
Open

Comments

@terion-name
Copy link

Subject

@lloy0076
Copy link
Contributor

lloy0076 commented Sep 4, 2017

@tabacitu - this is probably a request for enhancement but the wording is too vague and terse (and there's been no PR nor update to the issue) - I'm closing this - but please reopen if needed.

@lloy0076 lloy0076 closed this as completed Sep 4, 2017
@pavoltanuska
Copy link

I just ran into this issue and I wouldn't say it's a request for enhancement, I see it as bug.

Use case: I want to make a selector for days in week (Monday, Tuesday, Wednesday, ..., Sunday) - "Select days on which you want to send notifications".

I decided to use select_from_array field with options as array of days and enabled multiselect.

Issues:

  • I didn't find a way to specify the options array in a way, that would preserve names of the days as keys. This:
{
"name":"value",
"label":"Days",
"type":"select_from_array",
"options": [{"Monday": "Monday"}, {"Tuesday": "Tuesday"},{"Wednesday": "Wednesday"}],
"allows_null": false,
"default" : ["Monday", "Tuesday", "Wednesday"],
"allows_multiple": true
}

returns an error:

htmlspecialchars() expects parameter 1 to be string, object given (View: /project/vendor/backpack/crud/src/resources/views/fields/select_from_array.blade.php)

When specifying the options array as:

...
"options": ["Monday","Tuesday","Wednesday"]
...

the select is shown OK, but it doesn't preserve the keys and won't save correctly anyway (the conversion exception)

  • After saving, I get the Array to string conversion exception when saving the day(s) (one as well as multiple).

Is there a way to do this, which I didn't see/try (possible), or is it really a bug?

@lloy0076
Copy link
Contributor

lloy0076 commented Mar 8, 2018

The only way one could find out is to read the source, I guess:

  1. It may be the upstream CRUD source; or
  2. It might be something to do with this package.

In English, are you trying to do something like:

I want to have a setting where one can select multiple options (e.g. days) and the setting keys are the setting values that are stored but the values are the values that are displayed (e.g. 'mon' => 'Monday' -- the setting gets stored as mon but is displayed in the multi-select as Monday)

@pavoltanuska
Copy link

pavoltanuska commented Mar 8, 2018

Yes, your description is correct. I would just add, that having the keys stored (as provided in array) would be ideal, but I can manage without them (storing just the implicit keys, e.g. [0,1,3]). This is not possible either.

I indeed took a look at the code and I think the problem is that the Settings package just doesn't work well with arrays.

Normally, you'd specify the field's $casts on model, which isn't possible here, the values are always stored as plain strings. Since we're missing the $casts here, the Backpack\CRUD\PanelTraits\Update->decodeJsonCastedAttributes() method never gets called and the value has no way to be stored as JSON.

The only solution I see here is to store the value always as JSON - creating the $casts attribute on Setting model and refactor the SettingsServiceProvider's Config autofilling in that fashion.

But right now, I'm not sure whether it's possible to make it backwards compatible (my gut is telling me "yes"), so all suggestions are welcome.

@tabacitu
Copy link
Member

@terion-name , @pavoltanuska , @lloy0076 I just merged an update #69 that should solve the issue. When interpreting the field from the DB to the CRUD, we were casting it to array. Now we're json_decode()-ing it. That should allow you to specify an array in select2_from_array field type.

Let me know if it doesn't.
Cheers!

@pavoltanuska
Copy link

Unfortunately, it doesn't solve this issue.

Example 1

Let's say, I have a settings field with field column in the DB looking like this (because I intend to preserve the keys):

{"name":"value","label":"Days","type":"select2_from_array","options":[{"Monday":"Monday"},{"Tuesday":"Tuesday"},{"Wednesday":"Wednesday"}],"allows_null":false,"default":[],"allows_multiple":true}

It doesn't work, because the $field['options'] array looks like:

"options" => array:3 [▼
    0 => array:1 [▼
      "Monday" => "Monday"
    ]
    1 => array:1 [▼
      "Tuesday" => "Tuesday"
    ]
    2 => array:1 [▼
      "Wednesday" => "Wednesday"
    ]
  ]

and it dies with htmlspecialchars() expects parameter 1 to be string, array given, which is caused by the array being a level too deep.

Ok, that doesn't work. Let's try it without preserving the keys (making the options array one level deep).

Example 2

field column in the DB:

{"name":"value","label":"Days","type":"select2_from_array","options":["Monday","Tuesday","Wednesday"],"allows_null":false,"default":[],"allows_multiple":true}

Alright, it looks like it works, array is one level deep:

"options" => array:3 [▼
    0 => "Monday"
    1 => "Tuesday"
    2 => "Wednesday"
  ]

But!
What should we put as initial value to value column of this field? Let's say, we want all the days to be enabled by default.

  • Try no.1 - [0,1,2] - nope, just Monday is selected
  • Try no.2 - {1,2,3} - nope, just Monday is selected

Weird, shall we look at the $field['value']?

"value" => "[0,1,2]"

Why is this still a string? Because normally, we'd specify the field's casting on the model, which isn't something we can do on Settings model (without breaking the compatibility for other fields).

And that means two things:

  1. We are not able to show the initial values correctly
  2. We are not able to save the values correctly (THAT causes the array to string conversion error)

So actually, this issue has two sub-issues:

  • we are not able to generate any select* field with preserving the option keys
  • we are not able to show, nor store the correct multiple choice value

I have outlined the possible solution in #31 (comment), but there is no "5-minute" solution, as it could break the compatibility with other fields.

So @tabacitu, please reopen this issue. I'll try to come up with some backwards compatible solution, you are all welcome to help :)

@tabacitu
Copy link
Member

Wow - thank you for the comprehensive explanation @pavoltanuska .

Of course, reopened.

@tabacitu tabacitu reopened this Mar 19, 2018
@pitylee
Copy link

pitylee commented May 17, 2018

public function setup() { parent::setup(); // MUST HAVE, WITHOUT (LIKE WHEN GENERATED BY ARTISAN) IT WONT SAVE!

@tabacitu
Copy link
Member

Hi guys,

Following up on this. Tested and re-tested this. My conclusions:

  • the problem DOES NOT happen with multiple => false; so it only happens when we want a setting that has a select multiple (which makes sense, no way to store a PHP array in the DB);
  • the correct definition would be the one below (that would echo a correct SELECT item), but the SAVE will still not work;
{
	"name": "value",
	"label": "Days",
	"type": "select2_from_array",
	"options": {
		"Monday": "Monday",
		"Tuesday": "Tuesday",
		"Wednesday": "Wednesday"
	},
	"allows_null": false,
	"default": [],
	"allows_multiple": true
}

Right now I don't see a solution, other than having another field type, that actually stores a JSON in a hidden input... Will be back...

@ahmed402
Copy link

ahmed402 commented Nov 18, 2018

I came up with solution that is adding :
elseif(is_array($value)){ $request->request->set($key, json_encode($value)); }
in
vendor/backpack/crud/src/app/Http/Controllers/Operations/Update.php #56

@guil182
Copy link

guil182 commented Nov 30, 2018

Hi ahmed402,

I have the same problem : Array to string conversion (SQL: insert into appellation_type (appellation_id, type_id, updated_at, created_at) values (1, 2, 2018-11-30 11:02:02, 2018-11-30 11:02:02))

with a select2_multiple.

But I added your code to vendor/backpack/crud/src/app/Http/Controllers/Operations/CreateOperation.php and it don't work :/

@AnderPirozhkov
Copy link

AnderPirozhkov commented Jan 13, 2019

add this code to store and update methods before
$redirect_location = parent::storeCrud($request); in your crud controller
screenshot_1

@jayant-lumino
Copy link

How to do this setting then. I have created a seeder to on/off setting.

INSERT INTOsettings (id, key, name, description, value, field, active, created_at, updated_at`) VALUES (NULL, 'custom_job_poster', 'Job Posters can create their own regions', '', 'off', '{"name":"custom_region_by_job_poster","label":"Value","type":"select2_from_array","options":{"on":"On","off":"Off"}\r\n', '1', NULL, NULL);

On list it is working fine as expected
[https://prnt.sc/xpkr2r](Screenshot of list)

But when I tried to edit it. It throws me an error Undefined index: name

@chriskikoti
Copy link

I am having this issue in Backpack 5 as well, was this ever resolved?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants