laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.52k stars 11.02k forks source link

Soft delete ignored with exists validation rule #1820

Closed robclancy closed 11 years ago

robclancy commented 11 years ago

I would like to make sure a resource exists using the exists validation rule like normal however I would like to not include soft deleted resources. Since the database will only check against null it doesn't seem to work since the rule will use a string. For now I have hacked in...

$extra[$segments[$i]] = $segments[$i + 1] == '{NULL}' ? null : $segments[$i + 1];

into getExtraExistConditions and have my rule have {NULL} but obviously that isn't a good solution, wondering if something can be done about this?

ronaldcastillo commented 11 years ago

Maybe a less hacky solution is to use:

if( $this->db->connection($this->connection)->getSchemaBuilder()->hasColumn($collection, 'deleted_at') ) 
{
    $query->whereNull('deleted_at');
}

In Illuminate\Validation\DatabasePresenceVerifier::getCount() and getMultiCount() methods. And maybe even wrap the call to getSchemaBuilder() method (in a method called schema or similar).

Disclaimer: Haven't really tried it. I'll try it later and maybe make a PR. Don't know how to go around when the deleted_at column is renamed tho.

robclancy commented 11 years ago

Yeah that would work but would force it so it is always checking the ones that aren't soft deleted, sometimes people might want to check against them too.

However it gives me an idea of a better solution, in each method there is...

                foreach ($extra as $key => $extraValue)
        {
            $query->where($key, $extraValue);
        }

Perhaps this could be changed to (another hacky method)...

                foreach ($extra as $key => $extraValue)
        {
            if ($key == 'deleted_at' and empty($extraValue))
            {
                $query->whereNull($key);
                continue;
            }

            $query->where($key, $extraValue);
        }

The main issue really is that there is no way to imply null from the rules as a string as they could just as easily be empty strings. Which is why the check for the 'deleted_at' key is there.

None of these methods are good enough to get in, they are all hacky and have flaws. But hopefully something can be done.

ronaldcastillo commented 11 years ago

Maybe passing an extra parameter to the rule, something like: exists:table,column,include_deleted Where the default behavior would be to ignore deleted records.

taylorotwell commented 11 years ago

You can pass extra conditions to exists... passing "NULL" will do a whereNull check.

Like:

'exists:table,column,deleted_at,NULL'
robclancy commented 11 years ago

Thanks.

abishekrsrikaanth commented 10 years ago

This doesn't work. Throws an error column NULL not found

robclancy commented 10 years ago

Because you are doing it wrong. NULL goes in the value spot, you are putting it in the column spot. So you are doing exists:table,column,NULL instead of exists:table,column,null_column_check,NULL or something similar.

abishekrsrikaanth commented 10 years ago

so does this need to be changed on the docs as well? http://laravel.com/docs/validation#rule-exists Can you give me an example for how the rule should actually look?

robclancy commented 10 years ago

No? You are doing it wrong. The docs are fine. You would think column NULL not found is pretty clear on what you are doing wrong. Basically, you have the NULL on the column field, so it is not found.

abishekrsrikaanth commented 10 years ago

so if I wanted to check WHERE email='x' AND deleted_at=NULL. how should my rule look like. I am kinda confused..

robclancy commented 10 years ago

Nope docs are right. I need more sleep. (if you read the reply I just deleted, I quit life).

You do exists:table,email,deleted_at,NULL.

felixcheruiyot commented 10 years ago

For exists:table,email,deleted_at,NULL throws column NULL does not exist. One question, what does id in this example (from the doc) do?

'email' => 'unique:users,email_address,NULL,id,account_id,1'

In the above I would like to do

unique:table,email,NULL, deleted_at,deleted_at,NULL

and it works but doesn't make sense.

robclancy commented 10 years ago

I even told you the code! What the hell. exists:table,email,deleted_at,NULL. No where does it say to do exists:table,email,NULL,deleted_at. Are you high?

felixcheruiyot commented 10 years ago

Sure someone is high! @robclancy have you tried the above code? Or just writing. Here is the link to the doc http://laravel.com/docs/validation#rule-unique. Can you explain what is going on under "Adding Additional Where Clauses" or just leave it!

robclancy commented 10 years ago

Yes I have tried the code that is in the docs. I use it in a production environment, hence requesting the feature.

'user_id' => 'exists:users,id,deleted_at,NULL' will check if there is a user exists at the id and that deleted_at is not null (so they aren't soft deleted).

'user_id' => 'exists:users,id,deleted_at,NULL,type,douche' will check that a user at id exists with deleted_at null and type == 'douche'.

So for example if @anlutro was the user clearly the type would be douche and it would come back as a pass ;)

anlutro commented 10 years ago

Do note the difference in argument order for unique and exists, I don't know for what reason - it seems rather silly and apparently @robclancy is the one who suggested it.

exists:table,column,extra_column,1,extra_column_two,2 unique:table,column,1,extra_column,extra_column_two,2

robclancy commented 10 years ago

FUUUU: I PR'd it exactly the same: https://github.com/laravel/framework/pull/1263

robclancy commented 10 years ago

In the docs example. 'email' => 'unique:users,email_address,NULL,id,account_id,1'

First is table, second is column, third is excluded value, fourth is the excluded column, then the following pairs are your where conditions. So in this case fifth is the where column and sixth the where value.

The NULL in this case is because there is on exclusion going on.

abishekrsrikaanth commented 10 years ago

Wish you explained as clear as this before and I wish docs was this clear.

robclancy commented 10 years ago

Wish @anlutro never pissed me off on IRC and made me rage on issues ;) Blame him.

I'll PR a change to the docs.

milewski commented 8 years ago

I understand that this topic may be outdated, and some participants may no longer be active, but I believe it would make sense to set deleted_at to NULL by default.

While it's possible to encounter an SQL error if you attempt to check whether the deleted_at column exists, you could instead check if the model uses trait, by doing so, you could determine if the SoftDeletes trait is being used.

This would prevent users from falling into a trap, as I did. Initially, I assumed that adding the SoftDeletes trait would provide me with the necessary functionality. However, I later discovered that validation was broken in multiple areas when I attempted to run the same code twice. Unfortunately, I only discovered this issue after it was too late.

heroselohim commented 7 years ago

I know this is a really old topic, but I looked everywhere for something like this with no results. It m8 be of use to someone.

This is the laravel validator for Exists && Not Soft Deleted rule

exists_soft:user,id

\Validator::extend('exists_soft',
    function($attribute, $value, $parameters)
    {
        if (!isset($parameters[0])) {
            throw new \Exception('Validator "exists_soft" missing tablename.');
        }
        $tableName = $parameters[0];
        $columnName = isset($parameters[1])?$parameters[1]:null;
        $validator = \Validator::make([$attribute => $value],
        [
            $attribute => [
                Rule::exists($tableName, $columnName)
                    ->where(function ($query) {
                    $query->whereNull('deleted_at');
                }),
            ]
        ]);
        return $validator->passes();
    });
frankvice commented 6 years ago

I am going crazy with this, I tried:


            'number' => [
                'sometimes',
                'integer',
                Rule::unique(Episode::class, 'number')
                    ->ignore($episode->getIdAsString(), 'id')
                    ->where('season', $season->getIdAsString())
                    ->whereNot('deletedAt', null)
            ],

and

->whereNull('deletedAt')
and 

->whereNotNull('deletedAt')

It dones't work
I want it to ignore the records that were soft deleted
scofield-ua commented 6 years ago

@frankvice are you sure about deletedAt? It should be deleted_at by default (unless you changed it)?

Rule::unique('articles')->ignore($id)->whereNull('deleted_at')

did the trick for me.

bradley-varol commented 5 years ago

Works perfectly;

Rule::unique('users')
    ->ignore($userId)
    ->whereNull('deleted_at'),
fendis0709 commented 4 years ago

Thanks, @heroselohim . Your answer really helpful.

For those who want to implement these rule to global scope. Here's how to do it :

  1. Open file app/Providers/AppServiceProviders.php.
  2. Add heroselohim's answer to boot() method.
    public function boot(){
    // Put the code here
    }
  3. Done. Now you can call that rules in every form validation.
    'user_id' => ['exists_soft:users,id']

Tips If you want to customize validation message, you should do:

  1. Open resources/lang/en/validation.php.
  2. Add exists_soft to array validation. (I prefer copy from exists rule and change it to exists_soft)
    ...
    'exists' => 'The selected :attribute is invalid.',
    'exists_soft' => 'The selected :attribute is invalid.',
    ...
AkioSarkiz commented 3 years ago
Rule::unique("table", "column")->whereNull("deleted_at")
Qraxin commented 1 year ago

You can pass extra conditions to exists... passing "NULL" will do a whereNull check.

Like:

'exists:table,column,deleted_at,NULL'

You may also use the following construction, for me, it looks clearer:

Rule::exists(ClassName::class, 'id')->withoutTrashed()
mahmoud672 commented 1 year ago

You can pass extra conditions to exists... passing "NULL" will do a whereNull check.

Like:

'exists:table,column,deleted_at,NULL'

thx a lot it works perfectly

kamleshwebtech commented 3 weeks ago

For storing/adding record in database 'email' => 'required|email|unique:users,email,NULL,id,deleted_at,NULL',

For updating/editing record in database 'email' => 'required|email|unique:users,email,'.$id.',id,deleted_at,NULL', Here $id is record id of particular editing user in user table.