diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 83411030d1..a2896e79ca 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -143,8 +143,8 @@ class Handler extends ExceptionHandler ->withInput(); } - // This gets the MVC model name from the exception and formats in a way that's less fugly - $model_name = strtolower(implode(" ", preg_split('/(?=[A-Z])/', last(explode('\\', $e->getModel()))))); + // This gets the MVC model name from the exception and formats in a way that's less fugly + $model_name = trim(strtolower(implode(" ", preg_split('/(?=[A-Z])/', last(explode('\\', $e->getModel())))))); $route = str_plural(strtolower(last(explode('\\', $e->getModel())))).'.index'; // Sigh. @@ -160,9 +160,7 @@ class Handler extends ExceptionHandler $route = 'maintenances.index'; } elseif ($route === 'licenseseats.index') { $route = 'licenses.index'; - } elseif ($route === 'customfields.index') { - $route = 'fields.index'; - } elseif ($route === 'customfieldsets.index') { + } elseif (($route === 'customfieldsets.index') || ($route === 'customfields.index')) { $route = 'fields.index'; } diff --git a/app/Http/Controllers/CustomFieldsController.php b/app/Http/Controllers/CustomFieldsController.php index 7862719276..73e17f8942 100644 --- a/app/Http/Controllers/CustomFieldsController.php +++ b/app/Http/Controllers/CustomFieldsController.php @@ -144,10 +144,9 @@ class CustomFieldsController extends Controller */ public function deleteFieldFromFieldset($field_id, $fieldset_id) : RedirectResponse { + $this->authorize('update', CustomField::class); $field = CustomField::find($field_id); - $this->authorize('update', $field); - // Check that the field exists - this is mostly related to the demo, where we // rewrite the data every x minutes, so it's possible someone might be disassociating // a field from a fieldset just as we're wiping the database @@ -157,11 +156,12 @@ class CustomFieldsController extends Controller return redirect()->route('fieldsets.show', ['fieldset' => $fieldset_id]) ->with('success', trans('admin/custom_fields/message.field.delete.success')); } else { - return redirect()->back()->withErrors(['message' => "Field is in use and cannot be deleted."]); + return redirect()->back()->with('error', trans('admin/custom_fields/message.field.delete.error')) + ->withInput(); } } - return redirect()->back()->withErrors(['message' => "Error deleting field from fieldset"]); + return redirect()->back()->with('error', trans('admin/custom_fields/message.field.delete.error')); } @@ -172,20 +172,16 @@ class CustomFieldsController extends Controller * @author [Brady Wetherington] [] * @since [v1.8] */ - public function destroy($field_id) : RedirectResponse + public function destroy(CustomField $field) : RedirectResponse { - if ($field = CustomField::find($field_id)) { - $this->authorize('delete', $field); + $this->authorize('delete', CustomField::class); - if (($field->fieldset) && ($field->fieldset->count() > 0)) { - return redirect()->back()->withErrors(['message' => 'Field is in-use']); - } - $field->delete(); - return redirect()->route("fields.index") - ->with("success", trans('admin/custom_fields/message.field.delete.success')); + if (($field->fieldset) && ($field->fieldset->count() > 0)) { + return redirect()->back()->with('error', trans('admin/custom_fields/message.field.delete.in_use')); } - - return redirect()->back()->withErrors(['message' => 'Field does not exist']); + $field->delete(); + return redirect()->route("fields.index") + ->with("success", trans('admin/custom_fields/message.field.delete.success')); } @@ -198,7 +194,7 @@ class CustomFieldsController extends Controller */ public function edit(Request $request, CustomField $field) : View | RedirectResponse { - $this->authorize('update', $field); + $this->authorize('update', CustomField::class); $fieldsets = CustomFieldset::get(); $customFormat = ''; if ((stripos($field->format, 'regex') === 0) && ($field->format !== CustomField::PREDEFINED_FORMATS['MAC'])) { @@ -228,7 +224,7 @@ class CustomFieldsController extends Controller */ public function update(CustomFieldRequest $request, CustomField $field) : RedirectResponse { - $this->authorize('update', $field); + $this->authorize('update', CustomField::class); $show_in_email = $request->get("show_in_email", 0); $display_in_user_view = $request->get("display_in_user_view", 0); @@ -265,7 +261,6 @@ class CustomFieldsController extends Controller if ($field->save()) { - // Sync fields with fieldsets $fieldset_array = $request->input('associate_fieldsets'); if ($request->has('associate_fieldsets') && (is_array($fieldset_array))) { diff --git a/app/Policies/SnipePermissionsPolicy.php b/app/Policies/SnipePermissionsPolicy.php index cb1359fcd9..59c35394c5 100644 --- a/app/Policies/SnipePermissionsPolicy.php +++ b/app/Policies/SnipePermissionsPolicy.php @@ -85,7 +85,7 @@ abstract class SnipePermissionsPolicy } /** - * Determine whether the user can view the accessory. + * Determine whether the user can view the model. * * @param \App\Models\User $user * @return mixed @@ -101,7 +101,7 @@ abstract class SnipePermissionsPolicy } /** - * Determine whether the user can create accessories. + * Determine whether the user can create model. * * @param \App\Models\User $user * @return mixed @@ -112,7 +112,7 @@ abstract class SnipePermissionsPolicy } /** - * Determine whether the user can update the accessory. + * Determine whether the user can update the model. * * @param \App\Models\User $user * @return mixed @@ -124,7 +124,7 @@ abstract class SnipePermissionsPolicy /** - * Determine whether the user can update the accessory. + * Determine whether the user can update the model. * * @param \App\Models\User $user * @return mixed @@ -135,7 +135,7 @@ abstract class SnipePermissionsPolicy } /** - * Determine whether the user can delete the accessory. + * Determine whether the user can delete the model. * * @param \App\Models\User $user * @return mixed @@ -151,7 +151,7 @@ abstract class SnipePermissionsPolicy } /** - * Determine whether the user can manage the accessory. + * Determine whether the user can manage the model. * * @param \App\Models\User $user * @return mixed diff --git a/database/factories/UserFactory.php b/database/factories/UserFactory.php index 565621ca95..b9326fd83a 100644 --- a/database/factories/UserFactory.php +++ b/database/factories/UserFactory.php @@ -351,6 +351,17 @@ class UserFactory extends Factory return $this->appendPermission(['import' => '1']); } + public function createCustomFields() + { + return $this->appendPermission(['customfields.create' => '1']); + } + + public function viewCustomFields() + { + return $this->appendPermission(['customfields.view' => '1']); + } + + public function deleteCustomFields() { return $this->appendPermission(['customfields.delete' => '1']); diff --git a/resources/views/custom_fields/index.blade.php b/resources/views/custom_fields/index.blade.php index e94b7518c1..cf805e914e 100644 --- a/resources/views/custom_fields/index.blade.php +++ b/resources/views/custom_fields/index.blade.php @@ -84,15 +84,12 @@ @endcan @can('delete', $fieldset) -
- {{ method_field('DELETE') }} - @csrf + @if($fieldset->models->count() > 0) @else - + @endif -
@endcan @@ -237,9 +234,6 @@ -
- {{ method_field('DELETE') }} - @csrf @can('update', $field) @@ -249,19 +243,19 @@ @can('delete', $field) - @if($field->fieldset->count()>0) + @if ($field->fieldset->count() > 0) - @else - + @else + + + {{ trans('button.delete') }} + @endif @endcan -
diff --git a/tests/Feature/CustomFields/Ui/DeleteCustomFieldsTest.php b/tests/Feature/CustomFields/Ui/DeleteCustomFieldsTest.php new file mode 100644 index 0000000000..543122104f --- /dev/null +++ b/tests/Feature/CustomFields/Ui/DeleteCustomFieldsTest.php @@ -0,0 +1,69 @@ +actingAs(User::factory()->create()) + ->delete(route('fields.destroy', CustomField::factory()->create())) + ->assertForbidden(); + } + + + public function testCanDeleteCustomField() + { + $field = CustomField::factory()->create(); + $this->assertDatabaseHas('custom_fields', ['id' => $field->id]); + + $this->actingAs(User::factory()->deleteCustomFields()->create()) + ->delete(route('fields.destroy', $field)) + ->assertRedirectToRoute('fields.index') + ->assertStatus(302) + ->assertSessionHas('success'); + + $this->assertDatabaseMissing('custom_fields', ['id' => $field->id]); + } + + public function testCannotDeleteCustomFieldThatDoesNotExist() + { + + $response = $this->actingAs(User::factory()->viewCustomFields()->deleteCustomFields()->create()) + ->delete(route('fields.destroy', '49857589')) + ->assertRedirect(route('fields.index')) + ->assertSessionHas('error'); + + $temp = $this->followRedirects($response); + $temp->assertSee(trans('general.error'))->assertSee(trans('general.generic_model_not_found', ['model' => 'custom field'])); + + } + + public function testCannotDeleteFieldThatIsAssociatedWithFieldsets() + { + $field = CustomField::factory()->create(); + $fieldset = CustomFieldset::factory()->create(); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('fieldsets.associate', $fieldset), [ + 'field_id' => $field->id, + ]); + + $response = $this->actingAs(User::factory()->viewCustomFields()->deleteCustomFields()->create()) + ->from(route('fields.index')) + ->delete(route('fields.destroy', $field)) + ->assertStatus(302) + ->assertRedirect(route('fields.index')) + ->assertSessionHas('error'); + + $this->followRedirects($response)->assertSee(trans('general.error'))->assertSee(trans('admin/custom_fields/message.field.delete.in_use')); + + // Ensure the field is still in the database + $this->assertDatabaseHas('custom_fields', ['id' => $field->id]); + } +}