-
Notifications
You must be signed in to change notification settings - Fork 916
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
[Bug] Field relationship_select causes QueryException when validation fails. #4027
Comments
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:
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! -- |
On it. Thanks for the report @PitchRE ! I've just tested the Be back in a few! |
Hey @PitchRE Thanks again for taking time to submit the report. I think I found the culprit. I've submitted PR #4029 to address this issue. Please notice that the PR needs review and we are not sure yet if this is the correct/optimal way to fix this, but if you want to give it a try and let us know if it fixed for you, it could help us alot. I am going to close this since we have the PR, but feel free to comment here or re-open if the PR didn't fixed it for you. Wish you the best, |
Let's get back here @PitchRE ! Clearly not the same issue as in the other PR. I cannot reproduce your issue so .. let's try it again. Can you share with me your pivot table ? I will try to setup exactly the same scenario as you. |
@pxpm Please take a look at this repository. I just created brand new laravel project where I reproduced this issue. |
Hey @PitchRE Thanks again for your support helping to fix this issue. I am still not able to reproduce it. Here is a .gif of me playing with your repository: https://recordit.co/diqyWa58xQ Can you give me the exact steps to reproduce your issue ? Best, |
@pxpm Damn, I am speechless. If you cloned my repository, you for sure should be able to reproduce it. Please take a look at #4029 (comment) |
@PitchRE Hey there! Thanks for the submitted code. Unfortunatelly that would only apply to the selects fields and this is a general problem of using the combination with repeatable. Can you make sure you don't have the backpack files in your I don't see what could be if not that. Try clearing your caches Let me know, |
Yes, I am sure. Currently testing on fresh laravel project. PitchRE/issue-4027 4027.mp4I don't think I made any mistakes in relationship/field definition. It's only few files so you can verify that on that repo. Snippet I inserted bellow seems to fix it in my case. case 'array':
// check if field have `row_key` set, if it has we are in a repeatable
// and should get the current row value
if(isset($field['row_key'])) {
if (isset($current_value[$field['row_key']]) && isset($current_value[$field['row_key']][$field['name']])) {
$current_value = array($current_value[$field['row_key']][$field['name']]);
}
}
Log::channel('daily')->debug($current_value);
/// Check if array is multidimensional.
if (count($current_value) == count($current_value, COUNT_RECURSIVE))
{
/// Do as usual. Array contains only ids.
$current_value = $connected_entity
->whereIn($connected_entity_key_name, $current_value)
->get()
->pluck($field['attribute'], $connected_entity_key_name);
}
else
{
/// We Have to loop throught array because its array of arrays.
/// Then get $field['name'] from array and create array of ids.
$arr = array();
foreach ($current_value as $key => $value) {
array_push($arr, $value[$field['name']]);
}
$current_value = $connected_entity
->whereIn($connected_entity_key_name, $arr)
->get()
->pluck($field['attribute'], $connected_entity_key_name);
}
break; These are just my loose thoughts.So why sometimes CRUD::addField([
'name' => 'characters',
'label' => 'Assosciate Characters with this origins.',
'tab' => 'Relationships',
'wrapper' => [
'class' => 'form-group col-md-6',
],
'new_item_label' => 'Associate Character',
'init_rows' => 0,
'min_rows' => 0,
'max_rows' => 10,
]); In this case it will work because
But as soon as I change field definition to this, it breaks CRUD::addField([
'name' => 'characters',
'label' => 'Assosciate Characters with this origins.',
'tab' => 'Relationships',
'wrapper' => [
'class' => 'form-group col-md-6',
],
'pivot_selector' => [
'wrapper' => [
'class' => 'form-group col-md-6',
]
],
'pivotFields' => [
[
'name' => 'as',
'type' => 'text',
'wrapper' => [
'class' => 'form-group col-md-6',
],
],
],
'new_item_label' => 'Associate Character',
'init_rows' => 0,
'min_rows' => 0,
'max_rows' => 10,
]); And Log output is way different.
This multidimensional array. I guess It shouldn't be there at all ? |
@PitchRE that is due to the fact that the pivot selector is called #4032 should probably fix this without needing this fix in the field files. |
This is fixed I think. Please re-open or comment if I am wrong. Thanks for testing 4.2 @PitchRE , since this issue was opened alot of stuff have been added/fixed regarding relationships specially in the last days with nested relations. Let us know if you find something broken while using that branch :) Pedro |
Hey @pxpm, I can confirm that this issue still exists with relationship fields which includes subfields (pivot columns), with latest backpack/crud and backpack/pro |
Adding |
@csathoPeter are you getting this when using the 4.2 branch, or after upgrading to v5? |
@csathop-btu or @csathoPeter 👍 Thanks, can you provide me the output of Thanks |
Bump! Can anybody please give us more information on this? Would love to get to the bottom of this - are you still seeing this issue? |
@tabacitu I think this is fixed for long time now. I am going to close and re-open if someone pings here. Cheers |
Bug report
Refs;
Probably related to this PR: #4002
What I did
Created/Updated resource that contains values inside relationship_select field and validated request.
Validation rules doesn't have anything to do with relationship_select CRUD field.
Field Definition
What I expected to happen
If validation passes:
If validation fails:
What happened
This only occurs when validation fails and relationship_select field has any data.
Is it a bug in the latest version of Backpack?
After I run
composer update backpack/crud
the bug... is it still there?Backpack, Laravel, PHP, DB version
When I run
php artisan backpack:version
the output is:The text was updated successfully, but these errors were encountered: