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

Soft delete ignored with exists validation rule #1820

Closed
robclancy opened this issue Jul 5, 2013 · 29 comments
Closed

Soft delete ignored with exists validation rule #1820

robclancy opened this issue Jul 5, 2013 · 29 comments

Comments

@robclancy
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Member

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

Like:

'exists:table,column,deleted_at,NULL'

@robclancy
Copy link
Contributor Author

Thanks.

@abishekrsrikaanth
Copy link

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

@robclancy
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link

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

@robclancy
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

anlutro commented Jan 21, 2014

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
Copy link
Contributor Author

FUUUU: I PR'd it exactly the same: #1263

@robclancy
Copy link
Contributor Author

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
Copy link

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

@robclancy
Copy link
Contributor Author

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
Copy link

milewski commented Oct 14, 2016

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
Copy link

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
Copy link

frankvice commented Feb 16, 2018

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
Copy link

@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
Copy link

bradley-varol commented Apr 4, 2019

Works perfectly;

Rule::unique('users')
    ->ignore($userId)
    ->whereNull('deleted_at'),

@fendis0709
Copy link

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
}
  1. 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
Copy link

Rule::unique("table", "column")->whereNull("deleted_at")

@Qraxin
Copy link

Qraxin commented Mar 28, 2023

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests