Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3.11k stars 890 forks source link

select2_json_from_api wrong attribute to store behaviour #5517

Closed miquelangeld closed 4 months ago

miquelangeld commented 4 months ago

Bug report

What I did

I implemented the field like this:

$this->crud->addField([
            'label' => 'Booking', 
            'type' => 'select2_json_from_api',
            'name' => 'booking', 
            'data_source' => url('/admin/bookings/ajax-bookings-avalon-options'),
            'attribute' => 'id', 
            'attributes_to_store' => ['id'],
            'placeholder' =>'Select booking',
        ]);

the data source endpoint return this: [ { "id":"MURR230044076-1", "booking":"MURR230044076-1 Localizador: MUR0002611" } ]

So I expect that the value stored should be MURR230044076-1 but is storing this: {"id":"MURR230044076-1"} instead

Also if in the field I use different attributes: 'attribute' => 'booking', 'attributes_to_store' => ['id'],

then the select doesn't show any option

karandatwani92 commented 4 months ago

Hey @miquelangeld

select2_json_from_api is primarily designed for external APIs.

Your API source looks internal, so I recommend you use select2_from_ajax with Fetch Operation for the desired value.

miquelangeld commented 4 months ago

Hi @karandatwani92 although the url is internal, the data actually comes from an external system. But I have a class to query this data. I can't use select2_from_ajax, look this issue anyway if the endpoint return an object with the attribute id the stored value should be MURR230044076-1 not the json {"id":"MURR230044076-1"} isn't?

karandatwani92 commented 4 months ago

OK, I'm asking my colleague to guide here; he is the creator of this field. @promatik please help!

miquelangeld commented 4 months ago

Any chance to resolve this?

pxpm commented 4 months ago

Hello @miquelangeld sorry for the delay, this one is on me 🙏

The "main" purpose behind creating this field, was to consume external API's, so the main need is to store "at least" the id + any additional attributes you setup.

We discussed the possibility to support storing non-json in database, but that would deviate the field from the main purpose, plus it would only fit some use cases. Most likely you want to store an id, and show other attribute, not in your scenario, but if you think how most selects works, it display some user friendly string, while storing "machine strings/ids".

If we don't store both as json, when "editing" the entry, we would have only "the key", not the display value.

[
   'pending-review' => 'Pending Review',
   'isbn-001-az4' => 'Harry Potter Book 1', 
]

Your usecase is achievable with some extra step. You need to "trick" the field value, basically inverting what the field does. You decode the field json value before saving in the database, and store only the string you want. When you retrieve the entry from the database you encode the string in a format that the field is expecting.

CRUD::field('my_field') // ....
         ->events([
                   'saving' => function($entry) {
                         $entry->booking = json_decode($entry->booking?? [], true)['id'] ?? null;
                    },
                    'retrieved' => function($entry) {
                         $entry->booking = json_encode(['id' => $entry->booking]);
                    }
         ])

Doing this as model events only registered on the fields has the advantage over accessors/mutators because they will only happen when on a "crud context" where fields are used. When you retrieve your model with Model::find(id) you will still have a string when doing $model->booking vs if you did this as accessors/mutators you would get json from $model->booking.

Please note that while testing this solution (because I was not happy with the accessor messing with my model in general just to handle a field type), I found a bug with Backpack that I already fixed in 6.7.13, so please do a composer update backpack/crud before using this solution.

Let me know if that helps.

Cheers

miquelangeld commented 4 months ago

Hi @pxpm you're totally right and make perfect sense. It's a little bit embarrasing, but I didn't pay enough attention to the docs and the fact that the field store json by definition (I know it's even in the name select2_JSON_from_api) I close the issue. The field works perfectly. I've been waiting for this feature!! Thanks!! I also will close this one

pxpm commented 4 months ago

Hi @pxpm you're totally right and make perfect sense. It's a little bit embarrasing, but I didn't pay enough attention to the docs and the fact that the field store json by definition (I know it's even in the name select2_JSON_from_api) I close the issue. The field works perfectly. I've been waiting for this feature!! Thanks!! I also will close this one

Thank you @miquelangeld, I wouldn't be embarrassed, you pursued a valid use case I guess. It ended up with a bug fix, a docs update and your need satisfied. I would say it was win-win 👍

Thanks for your time and patience 🙏