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

[v5][Summary] Not all fields work as subfields inside relationship #4109

Closed
tabacitu opened this issue Jan 22, 2022 · 8 comments
Closed

[v5][Summary] Not all fields work as subfields inside relationship #4109

tabacitu opened this issue Jan 22, 2022 · 8 comments

Comments

@tabacitu
Copy link
Member

tabacitu commented Jan 22, 2022

Right now, we do NOT have a clear map:

  • what field types works as subfields inside repeatable?
  • what field types work as subfields inside relationship?
  • is there any difference between the two?

And we should. We MUST. It's just a matter of time before somebody tries a combination we haven't - and they will get a horrible experience. If we do not tell them "X does not work as subfield", they will expect everything to work as subfields. So we need to TEST EVERYTHING, KNOW what works and what doesn't and DOCUMENT what works and what doesn't. Then, one by one, we can make them work (if it's reasonable to do so). Here's my first attempt to test EVERYTHING.

What is left untested?

  • all subfields inside repeatable field - we have them inside the Dummies CRUD ✅
  • all subfields inside relationship field
    • (A) work on the 1-1 interface (attributes on hasOne/morphOne) 🟨
    • (B) work on the 1-n interface (attributes on hasMany/morphMany) 🟨
    • (C) work on the n-n interface (attributes on belongsToMany/morphToMany pivot table) 🟨

So what we need to add to the Demo, to have all bases covered, are examples that use all field types as subfields, inside the yellow scenarios above. The new interfaces we just added for the relationship field. Fortunately, we already have a model with ALL field types. And they're already configured inside MonsterCrudController, we can just re-use their definition like we did in Dummies CRUD.

How do we test the untested?

We can add three new entities, Cave, Story and Hero, with the following db tables:

Screenshot 2022-01-22 at 19 25 40

This means that:

  • Cave hasOne Monster (there's a cave_id on the monsters table) - treats use case (A)
  • Story hasMany Monster (there's a story_id on the monsters table) - treats use case (B)
  • Hero hasMany Monster (there's a hero_id on the monsters` table, which also means:
    • Hero belongsToMany Story (through the monsters pivot table) - treats use case (C)
    • Story belongsToMany Hero (through the monsters pivot table)- treats use case (C)

So we can build three new CRUDs, to have ALL subfields tested in these 3 new use cases. I'll submit a PR to the demo with these three new CRUDs:

Screenshot 2022-01-22 at 19 28 37
Screenshot 2022-01-22 at 19 31 11
Screenshot 2022-01-22 at 19 31 27

Update: PR here - Laravel-Backpack/demo#346

@tabacitu
Copy link
Member Author

tabacitu commented Jan 22, 2022

Now let's see which ones work and which ones don't... I'll update this table as I test...

My tests have been done with the following branches merged:

Legend:
⛔️ - not allowed (doesn't work, but that's expected)
❌ - not working (but probably should)
✅ - working
⚠️ - working with caveats / tricks / only in certain circumstances

Can I use this field type as a subfield?

Field Type Repeatable (demo: Dummies) 1-1 relationship (demo: Caves) 1-n relationship (demo: Stories) n-n relationship (demo: Heroes)
address_algolia ❌ bug on save
address_google ? ? ?
browse
browse_multiple
base64_image
checkbox
checklist ?
checklist_dependency ⛔️ ⛔️ ⛔️ ⛔️
ckeditor
color
color_picker
custom_html
date
date_picker
date_range #4140 #4140 ?
datetime
datetime_picker
easymde
email
enum ⛔️ ? ? ?
hidden
icon_picker
image ?
month
number
password
radio ?
range
relationship ⚠️ ⚠️ w/o subfields ⚠️ w/o subfields ⛔️#4142
repeatable ⛔️ ⛔️ ⛔️ ⛔️
select (1-n relationship) ⛔️#4142
select_grouped ⛔️#4142
select2 (1-n relationship) ⛔️#4142
select_multiple (n-n relationship) ⛔️#4142
select2_multiple (n-n relationship) ⛔️#4142
select2_nested ⛔️#4142
select2_grouped ⛔️#4142
select_and_order ⛔️#4142
select_from_array
select2_from_array
select2_from_ajax ⛔️#4142
select2_from_ajax_multiple ⛔️#4142
summernote
table
text
textarea
time
tinymce
upload ⛔️ #4141 #4141 ?
upload_multiple ⛔️ #4141 #4141 ?
url
video
view
week
wysiwyg

Can I use this field feature inside a subfield?

Field Type Repeatable 1-1 relationship 1-n relationship n-n relationship (pivot)
array syntax
fluent syntax ⛔️ ⛔️ ⛔️ ⛔️
string syntax ?
fake fields ?
tabs ⛔️ ⛔️ ⛔️ ⛔️
identifiable attribute

@tabacitu tabacitu changed the title [4.2][Bug][WIP] Not all fields work as subfields inside relationship [4.2][Summary][WIP] Not all fields work as subfields inside relationship Jan 23, 2022
@tabacitu
Copy link
Member Author

tabacitu commented Feb 2, 2022

@tabacitu
Copy link
Member Author

tabacitu commented Feb 2, 2022

I'm very happy with the progress on this. I've opened issues for the few cases I think we should fix before launch. Other than that... I'm comfortable with moving this to the 4.2.x board 🎉 to make it a "problem for future us".

@tabacitu
Copy link
Member Author

tabacitu commented Feb 10, 2022

Round-up. There are still a few things to be done here, to consider the issue completely closed:

@tabacitu tabacitu changed the title [4.2][Summary][WIP] Not all fields work as subfields inside relationship [v5][Summary] Not all fields work as subfields inside relationship Feb 10, 2022
@kee0624
Copy link

kee0624 commented Mar 24, 2022

Hi, I recently just updated my laravel backpack from 4.1 to 5, and try to add the nested repeatable under the repeatable, there is an issue here like I am not allowed to click on the nested "New Item" on the nested repeatable field, but if I create the post and edit it later, I am allowed to click on the "New Item" button and it shows the nested repeated field, is it a bug or intended? And I saw the above table it said that the repeatable are not allowed to use as a subfield, so I am confused here, it should not be working, but why it is working when I edit the post.

below is my crud fields for reference.
$this->crud->addFields([ [ 'name' => 'services', 'label' => 'Services', 'type' => 'repeatable', 'subfields' => [ [ 'name' => 'title', 'label' => 'Title', ], [ 'name' => 'buttons', 'label' => 'Buttons', 'type' => 'repeatable', 'subfields' => [ 'name' => 'buttons_label', 'label' => 'Label', ], ], ], ], ]);

@pxpm
Copy link
Contributor

pxpm commented Mar 24, 2022

Hello @kee0624 . It migth display on page, but surelly it's not working.

Subfields inside subfields are not supported. Some will display, some will not display and throw errors. Use at will, we do not recommend and we do not support them (for now).

thanks

@guleswine
Copy link
Contributor

guleswine commented May 21, 2022

                [   // Checklist
                    'name'        => 'sources',
                    'label'       => "Источники",
                    'type'        => 'select2_from_array',
                    'options'     => Source::where('removed',false)->get()->pluck('name','id'),
                    'default'     => Source::where('removed',false)->get()->pluck('id')->toArray(),
                   // 'allows_null' => false,
                    'allows_multiple' => true,
                    'wrapper' => [
                        'class' => 'form-group col-md-3',
                    ],
                ],

select_from_array

Hey guys, I noticed this strange behavior
its subfields of relationship

@pxpm
Copy link
Contributor

pxpm commented May 23, 2022

Hello @guleswine it's indeed strange but I guess that it might be some corner case while using default values or something like that. As far as I am aware select2_from_array was working properly as a subfield.

If you remove the default does the problem still happen ?

If so, please open a separate issue so we can have a look at and don't forget about it in the middle of this thread.

Thanks

Repository owner moved this from Todo to Done in Backpack v5.1 Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants