Laravel-Backpack / CRUD

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

[Bug] Errors for relationship subfields are not detected to set the active tab correctly #5686

Open krjohnston opened 4 days ago

krjohnston commented 4 days ago

Bug report

What I did

Added tabbed panels for relationship with subfields in setupUpdateOperation

CRUD::field('inspectorAllocations')
    ->type('relationship')
    ->subfields([
        [
            'name' => 'inspector'
            'ajax' => true,
            'attribute' => 'reverse_full_name',
        ],
        [
            'name' => 'valid_date',
            'type' => 'date',
    ])
    ->tab('Inspector Allocations');

What I expected to happen

When the first error is on a relationship subfield, I expect that tab to be the active tab

What happened

The subfield was not recognised because the crud field in the array returned from $crud->getCurrentFields() is inspectorAllocations but the $fieldName under test is inspectorAllocations.0.name.

What I've already tried to fix it

I published show_tabbed_fields.blade.php and added a preg_split to obtain the parent name of any subfields before checking the array of available $fieldNames This works for my case and does not have any adverse effect on standard format fieldNames

@php
    $horizontalTabs = $crud->getTabsType()=='horizontal' ? true : false;
    $tabWithError = (function() use ($crud) {
        if(! session()->get('errors')) {
            return false;
        }
        foreach(session()->get('errors')->getBags() as $bag => $errorMessages) {
            foreach($errorMessages->getMessages() as $fieldName => $messages) {

                # Transform subfield $fieldNames into the correct CRUD fieldName format 
                # preg_split by the array separator .digits. and take only the first element
                $fieldName = preg_split('/\.\d+\./',$fieldName)[0];

                if(array_key_exists($fieldName, $crud->getCurrentFields()) && array_key_exists('tab', $crud->getCurrentFields()[$fieldName])) {
                    return $crud->getCurrentFields()[$fieldName]['tab'];
                }
            }
        }
        return false;
    })();
@endphp

Is it a bug in the latest version of Backpack?

After I run composer update backpack/crud I am on 6.7.39 and the bug is still there.

Backpack, Laravel, PHP, DB version

When I run php artisan backpack:version the output is:

PHP VERSION:

8.2.18

PHP EXTENSIONS:

Core, date, libxml, openssl, pcre, zlib, filter, hash, json, pcntl, random, Reflection, SPL, session, standard, sodium, mysqlnd, PDO, xml, bcmath, calendar, ctype, curl, dom, mbstring, FFI, fileinfo, ftp, gd, gettext, iconv, igbinary, imagick, imap, intl, ldap, exif, msgpack, mysqli, pcov, pdo_mysql, pdo_pgsql, pdo_sqlite, pgsql, Phar, posix, readline, redis, shmop, SimpleXML, soap, sockets, sqlite3, sysvmsg, sysvsem, sysvshm, tokenizer, xmlreader, xmlwriter, xsl, zip, memcached, swoole, Zend OPcache, xdebug

LARAVEL VERSION:

10.48.3.0

BACKPACK PACKAGE VERSIONS:

backpack/basset: 1.3.0 backpack/crud: 6.7.39 backpack/devtools: 3.1.0 backpack/editable-columns: 3.0.8 backpack/generators: v4.0.4 backpack/permissionmanager: 7.2.1 backpack/pro: 2.1.11 backpack/theme-tabler: 1.2.8

welcome[bot] commented 4 days 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:

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

karandatwani92 commented 6 hours ago

Hey @krjohnston

Yes, It does not take to the error field tab, but it highlights the tab where the error is with the details on top:

Screenshot 2024-10-13 at 8 35 28 PM

krjohnston commented 5 hours ago

I agree the error is shown, but this is inconsistent behaviour compared to all other fields. My client insisted that I rectify this which I did, however I believe this is a bug that ought to be addressed in the core.

On Sun, 13 Oct 2024, 16:06 Karan Datwani, @.***> wrote:

Hey @krjohnston https://github.com/krjohnston

Yes, It does not take to the error field tab, but it highlights the tab where the error is with the details on top:

Screenshot.2024-10-13.at.8.35.28.PM.png (view on web) https://github.com/user-attachments/assets/4024d9dc-2b96-46d3-a17f-335ae4d7d372

— Reply to this email directly, view it on GitHub https://github.com/Laravel-Backpack/CRUD/issues/5686#issuecomment-2409014198, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCDJIANCPGJJRYMVJ3B2H3Z3KEAFAVCNFSM6AAAAABPUB5LKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBZGAYTIMJZHA . You are receiving this because you were mentioned.Message ID: @.***>