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

Fixed #8926, #8252 - introduce circular reference check for location parent_id - rebased from #8253 #8927

Merged
merged 8 commits into from
Dec 19, 2020
2 changes: 0 additions & 2 deletions app/Http/Controllers/LocationsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ public function store(ImageUploadRequest $request)
{
$this->authorize('create', Location::class);
$location = new Location();
$location->id = null; // This is required to make Laravels different validation work, it errors if the parameter doesn't exist (maybe a bug)?
$location->name = $request->input('name');
$location->parent_id = $request->input('parent_id', null);
$location->currency = $request->input('currency', '$');
Expand Down Expand Up @@ -132,7 +131,6 @@ public function update(ImageUploadRequest $request, $locationId = null)
return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist'));
}


// Update the location data
$location->name = $request->input('name');
$location->parent_id = $request->input('parent_id', null);
Expand Down
2 changes: 1 addition & 1 deletion app/Models/Location.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Location extends SnipeModel
'address2' => 'max:80|nullable',
'zip' => 'min:3|max:10|nullable',
'manager_id' => 'exists:users,id|nullable',
'parent_id' => 'nullable|exists:locations,id|different:id',
'parent_id' => 'non_circular:locations,id'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm that this will work with nulled parent_ids? Laravel changed the way their validator worked a while back where you have to specify nullable if it is nullable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Other than that, this looks great to me :) )

);

protected $casts = [
Expand Down
46 changes: 46 additions & 0 deletions app/Providers/ValidationServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,52 @@ public function boot()
});


// Prevent circular references
//
// Example usage in Location model where parent_id references another Location:
//
// protected $rules = array(
// 'parent_id' => 'non_circular:locations,id,10'
// );
//
Validator::extend('non_circular', function ($attribute, $value, $parameters, $validator) {
if (count($parameters) < 2) {
throw new \Exception('Required validator parameters: <table>,<primary key>[,depth]');
}

// Parameters from the rule implementation ($pk will likely be 'id')
$table = array_get($parameters, 0);
$pk = array_get($parameters, 1);
$depth = (int) array_get($parameters, 2, 50);

// Data from the edited model
$data = $validator->getData();

// The primary key value from the edited model
$data_pk = array_get($data, $pk);
$value_pk = $value;

// If we’re editing an existing model and there is a parent value set…
while ($data_pk && $value_pk) {

// It’s not valid for any parent id to be equal to the existing model’s id
if ($data_pk == $value_pk) {
return false;
}

// Avoid accidental infinite loops
if (--$depth < 0) {
return false;
}

// Get the next parent id
$value_pk = DB::table($table)->select($attribute)->where($pk, '=', $value_pk)->value($attribute);
}

return true;
});


// Yo dawg. I heard you like validators.
// This validates the custom validator regex in custom fields.
// We're just checking that the regex won't throw an exception, not
Expand Down
1 change: 1 addition & 0 deletions resources/lang/en/validation.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
'uploaded' => 'The :attribute failed to upload.',
'url' => 'The :attribute format is invalid.',
"unique_undeleted" => "The :attribute must be unique.",
"non_circular" => "The :attribute must not create a circular reference.",

/*
|--------------------------------------------------------------------------
Expand Down