CRUD icon indicating copy to clipboard operation
CRUD copied to clipboard

[Bug] Validate upload_multiple field as sometimes and required

Open AmrSubZero opened this issue 4 years ago • 6 comments

Bug report

What I did

I'm trying to validate upload_multiple field as sometimes, only required if there aren't any existing items :

  • if there's items selected to be uploaded, then proceed.
  • if nothing selecting, and there're existing items, proceed also.
  • But if there aren't any selected items and no items existing (uploaded before), then show the required field error message.

Debugging the post request, assuming the field name is photos[] it always having the first item as null :

[
    "photos" => [
        0 => null
    ]
]

validating using sometimes|required

"photos" => "between:1,4",
"photos.*" => "sometimes|required"

What I expected to happen

it should always care about existing uploaded items in the validation, see if there's existing items uploaded then do not show required error message.

if no existing items uploaded and the submit request has empty photos then show the required error.

What happened

if an item(s) selected, it's proceeding and uploading them. now when i try to submit the form without selecting any items (by knowing there are existing items uploaded before), it's showing the required field error message.

What I've already tried to fix it

because the photos Array always having the first item as null if nothing selected as i mentioned above.

A solution to deal with this, is to provide nullable to the validation rule :

"photos.*" => "sometimes|nullable|..."

But with nullable set to rules, the required rule will never work.

Is it a bug in the latest version of Backpack?

After I run composer update backpack/crud the bug... is it still there?

Yes

Backpack, Laravel, PHP, DB version

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

### PHP VERSION:
PHP 7.4.13

### LARAVEL VERSION:
v8.46.0

### BACKPACK VERSION:
4.1.46

AmrSubZero avatar Jul 04 '21 03:07 AmrSubZero

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:

  • Bug Reports, Feature Requests - Github Issues (here);
  • Quick help (How do I do X) - Gitter Chatroom;
  • Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the backpack-for-laravel tag;
  • Showing off something you've made, asking for opinion on Backpack/Laravel matters - Reddit;

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

welcome[bot] avatar Jul 04 '21 03:07 welcome[bot]

@AmrSubZero

I have been wandering for a solution for this. But without major changes in the field inner workings I don't think it's possible.

So for now, you should implement a custom solution for your use-case. What I mean, just setup a validator rule for your upload field and use it for validation. https://laravel.com/docs/8.x/validation#custom-validation-rules

Let me know if you have any issues with implementing the rule, but you know the requirements it should be straightfoward.

I will leave this open for a while.

Let me know how it goes, Pedro

pxpm avatar Aug 28 '21 09:08 pxpm

Thanks for letting us know @AmrSubZero - I had no idea this was a thing. If you do end up creating a custom rule for this, please share the code here, it'll definitely help others too.

Actually now that I think about it... we could provide this rule in Backpack itself too... So that it's easier for people to use it... But since this is the first I hear about it... I assume not many people need this so a copy-paste from here might strike the best balance...

Did you manage to create the Custom Validation Rule @AmrSubZero ? Need a hand?

Cheers!

tabacitu avatar Sep 02 '21 13:09 tabacitu

yes i will create the custom validation rule as you guys suggested, and see if things work, if i ended up with something stable i will post it here.

AmrSubZero avatar Sep 11 '21 02:09 AmrSubZero

@tabacitu, just my two cents, I had a similar problem 1 year ago, https://github.com/Laravel-Backpack/CRUD/pull/3278. I think maybe on 4.2 we should really think about ensuring a standard behavior for all fields ✌ including, in this case, not send unfilled fields.

promatik avatar Sep 26 '21 22:09 promatik

I think I am seeing the same issue arise with the browse_multiple field, i.e. when no changes have been made, on form submit the field value is not POSTed and therefore fails validation (or if the field is not required, all existing selections are effectively deleted).

In my case, the field is truly optional -- but if files have been selected, they should remain there unless specifically removed.

@AmrSubZero - did you ever create a custom validation rule to resolve this, and if so, what did that wind up looking like?

tibbsa avatar Jan 11 '22 15:01 tibbsa

Fixed by #5038 in v6 🎉 In a really nice, clean way.

tabacitu avatar Apr 29 '23 10:04 tabacitu