Closed rroblik closed 2 years ago
Hello there! Thanks for opening your first issue on this repo!
Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.
Backpack communication channels:
backpack-for-laravel
tag;Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.
Thank you!
-- Justin Case The Backpack Robot
Hey @rroblik ,
Thanks for reaching out.
I wouldn't quite call this a "bug". It's the way web forms work, so it's the way Backpack works - you can only have one field with a certain name per page. So you can't add two "settings" fields. I know this isn't news to you - I see you've found quite a nifty solution with our fake fields, even though they weren't designed to be used that way. What I think would help you is this idea here, basically being able to define the exact place it would be saved: 'store_in' => 'settings.option_a'
. It's something we're considering for the next version of Backpack, so if you think it's a good idea please give a thumbs up or reply there too.
I didn't find where values as casted to array
Indeed, when you use Backpack's fake fields, Backpack will "force" casting it to JSON (even if settings
is not in your $casts
), just to make it easier for people who have forgotten to do so.
To help you take your project further now, though - maybe this helps:
Solution A) You can add an accessor an mutator on that model, to customize how settings
gets saved.
Solution B) Alternatively, you can use model events in your CrudController's setupCreateOperation()
and/or setupUpdateOperation()
methods, to customize the Model at certain points. For example:
public function setupCreateOperation()
{
// this will only get triggered inside the Create operation because that's where we've defined it
// but if your setupUpdateOperation() calls setupCreateOperation(), it'll run there too;
// alternatively, you can use different events: Product::creating() and Product::updating()
Product::saving(function ($entry) {
// TODO: merge request()->input('option_a') and request()->input('option_b') to a single JSON array and
// store it on $entry->settings, basically customizing the save process
});
}
I'm going to close the issue since
Let me know if you think I'm wrong or I misunderstood you issue - like all people, I can be wrong all the freakin' time 😀
Cheers!
@tabacitu Thanks for the complete (and quick) answer. I was on my mobile phone to redact this issue ; so I will add some information now with real code example because maybe there is a misunderstood 😄
# AppCrudController.php
# ...
protected function setupCreateOperation()
{
CRUD::addField([
'name' => 'git_app',
'label' => 'Git app',
'type' => 'table',
'columns' => [
'pid' => 'Project ID',
'pipeline_token' => 'Pipeline Token'
],
'store_in' => 'ci_settings',
'store_as_json' => true,
'fake' => true,
'min' => 1,
'max' => 1,
'orderable' => false
]);
CRUD::addField([
'name' => 'git_src',
'label' => 'Git sources',
'type' => 'table',
'columns' => [
'pid' => 'Project ID',
'path' => 'Project Path',
],
'store_in' => 'ci_settings',
'store_as_json' => true,
'fake' => true,
'max' => 1,
'min' => 1,
'orderable' => false
]);
CRUD::addField([
'name' => 'ci',
'label' => 'CI settings',
'type' => 'table',
'columns' => [
'deploy_app_ref' => 'App reference',
'deploy_token' => 'Deploy token',
],
'store_in' => 'ci_settings',
'store_as_json' => true,
'fake' => true,
'max' => 1,
'min' => 1,
'orderable' => false
]);
}
# Models/App.php
# ...
class App extends Model
{
# ...
protected $fillable = [
'ci_settings',
];
protected $casts = [
'ci_settings' => 'array',
];
protected $fakeColumns = [
'ci_settings'
];
}
# migration
class CreateAppsTable extends Migration
{
/**
* Run the migrations.
*
* @return void
*/
public function up()
{
Schema::create('apps', function (Blueprint $table) {
#...
$table->json('ci_settings')->nullable();
#...
});
}
# ...
}
ci_settings
field value in database :
{"git_app":"[{\"pid\":\"12\",\"pipeline_token\":\"ptoken\"}]","git_src":"[{\"pid\":\"666\",\"path\":\"foo\/bar\"}]","ci":"[{\"deploy_app_ref\":\"deploy\",\"deploy_token\":\"dtoken\"}]"}
That the main point : my data should NOT be array []
; because I forced max to 1
.
The optionnal point is (in orange on picture) : the "Add" button shouldn't be displayed is such field configuration
I will look at your "workaround" ; solution B seems good but I want to do such "formatting" at the lower level possible (maybe solution A in fact)...
Thanks
Hmm... it looks like we have a bug here indeed: the "+ Add" button shouldn't be visible when you've reached the max
(inside a table
field). I've added a separate issue about it here - https://github.com/Laravel-Backpack/CRUD/issues/4076 so we deal with that. Thank you 🙏
However, I still don't think we should convert the array of arrays
into a single array
, when there's only one item. It's useful in your particular use case, but it'll be VERY confusing for other devs and in other scenarios. The result of that field should always be the same type - an array of arrays.
Think about it this way... what happens when there's a table
with multiple entries possible... but then the admin removes some of them and reaches ends up with just one entry? Should the data type change to a simple array then? No, cause you'd be changing the data type in the db 😀
Don't get me wrong, I understand why it makes sense from your point of view, as a developer of this application. I agree, it would've been cool for you if table
worked that way and switched data types depending on how many entries there are. But from other developers' point of view, what you're proposing is unexpected.
I think the confusion here is rooted in the fact that you're using table
in a way that wasn't intended - to accept ONE entry, instead of multiple. And then merging three table
fields into one. But... it's entire "raison d'etre" is to accept multiple entries. And I don't we should change data type for when it's used in a different way than intended, sorry.
Feel free to convince me otherwise if you think I'm wrong. It happens 😀
Maybe we can think about a simple table_single
field type ; derivated from table
: same UI (with #4076 fix ;)) but different behavior on save/load : single row of data. Best of 2 worlds !
Hey guys! 🌞
I just have one question:
I mean, don't get me wrong, but from the table
you are just using the "interface". It's not just the "Add" button, you have "reorder" buttons to re-order a single element too. Does it make sense at all? Why not setup the fields directly ??
CRUD::field('table_1_header')->type('custom_html')->value('<h3>Header for my fields all in the same row</h3>');
CRUD::field('table1_field1')->type('text')->wrapper(['class' => 'col-md-4'])->fake(true);
CRUD::field('table1_field2')->type('text')->wrapper(['class' => 'col-md-4'])->fake(true);
CRUD::field('table_2_header')->type('custom_html')->value('<h3>Header for my fields all in the same row</h3>');
CRUD::field('table2_field1')->type('text')->wrapper(['class' => 'col-md-4'])->fake(true);
You will endup with this in your database:
{"table1_field1":"hey","table1_field2":"oi","table2_field1":"sup ","table2_field2":"dup"}
Is there any problem using them like this @rroblik that I am missing ??
Thanks for the feedback, Pedro
@pxpm you're totally right it look like the cleanest possible option.
2 notes :
{"table1":{"field1":"hey","field2":"oi"}}
(that what I "need" to "group" related data under a same JSON key)CRUD::field('table_1_header')->type('custom_html')->value('<h3>Header for my fields all in the same row</h3>');
; calling an html tag a "field" is not verry sexy ; but maybe later we will have a CRUD::htmlElement
or so function ! Anyway I will try to mix @pxpm answer with @tabacitu B (or A) solution, to get best of two world !
I will update this issue after, just for sharing.
Thanks
Of course, if a developer is the one that is an admin, you can always use a JSON editor directly - https://github.com/ziming/json-field-for-backpack
Hey @rroblik thanks for the feedback, I don't like it much either, but I convinced myself that the field is custom_html
and it is what it is, a custom html field... it works to don't worry much about it and usually gets the job done 🙃
You can also split the ci_settings
in the three columns you need in the database so each one stores the corresponding values and use an acessor for ci_settings
model attribute, adding it to $appends
. That way you would still call $model->ci_settings
and get the result of your acessor where you combine the three other model attributes in the array form you need.
Or .. keep everything inside the same column as I said first and use the acessor / appends technique to return the results parsed the way you need them.
It also seems reasonable to me that a custom field like table_from_array
, a sligth variation of the table field (strip the add more, orders etc) and could go something along this lines:
CRUD::field('db_column')->type('table_from_array')->structure(
[
'table_1' => [
'fields' => [
[
'name' => 'table1_field1'
],
[
'name' => 'table1_field1'
],
],
'label' => 'table 1 title',
],
'table_2' => [
'fields' => [
[
'name' => 'table2_field2'
],
[
'name' => 'table2_field2'
],
],
'label' => 'table 2 title',
],
]
)
Where the tables
would be the keys and fields the values to save in the db. Note that in table
field you can only use text
fields, while in repeatable
https://backpackforlaravel.com/docs/4.1/crud-fields#repeatable-1 you can use almost all crud fields, so take that into consideration if you endup choosing one to start working as base.
Best, Pedro
Bug report
What I did
Use 2
table
fields type to store each time one item with simple key/value values, in a singlesettings
Model attribute (json field on db side). Set ˋmax` to 1 to allow only one row on for each.For example :
What I expected to happen
Stored value in simple JSON object
What happened
Stored each value as array
Also No "add XX" input must be displayed
What I've already tried to fix it
As the main goal of using
table
in this situation is have a cleaner UI/UX without any blade custom I looked for fieldset support but only dirty solutions was found, even in #3283. Grouping fields is a must have when creating a lot a fields form, without requiring custom templating.I didn't find where values as casted to array but if I manually update json in DB, then backpack cannot proper reload value in table cells when editing the entry using UI.
single row table mean we want to store flat value, not array value
Is it a bug in the latest version of Backpack?
yes, 4.1
Backpack, Laravel, PHP, DB version
When I run
php artisan backpack:version
the output is: