Crud: [4.1][Feature] Custom error message in case destroy() failed

[pavoltanuska]

Right now there's just one default message in case the destroy() method failed.

I'm using some nesting/relations and I want to prevent the user from deleting the resource in case it has some specific relations still defined.

From functional point of view, everything works ok. I'm overriding the destroy() method in the CrudController, doing the checks and failing the destroy if there's a problem. But right now, there's no way to tell the user about the specifics of the problem.

My approach would be to rewrite said part of list.blade.php slightly, to:

error: function(result) {
    // Show an alert with the result
    var notificationText = "{{ trans('backpack::crud.delete_confirmation_not_message') }}";

    // Is there anything custom we'd like to tell the user?
    if (result.responseJSON.text != undefined) {
        notificationText = result.responseJSON.text;
    }

    new PNotify({
      title: "{{ trans('backpack::crud.delete_confirmation_not_title') }}",
      text: notificationText,
      type: "warning"
    });
}

And with this, we are free to throw 409 (or some other 4/5 code, if applicable) in the destroy() method with some text. (e.g. return response()->json(['text' => $error)], 409);)

It shouldn't be a breaking change.

I don't know if something like this isn't a part of some other plans/PRs, so forgive me for not being informed, if that's the case :)

Is it worth a PR?

[lloy0076]

I'd prefer to keep the notify in the PHP controller, if possible, similar to the update messge (and the message if delete occurs).

@tabacitu - what say you?

[tabacitu]

Hi guys. This sounds like a good improvement, yes. I agree!

[slickorange]

Hi, @tabacitu any update on this? Would be great if we could change the message. I have the exact same use case as @pavoltanuska

[jrpub]

Same issue.

I looked around, and one of the issue is that:

• For create and edit, the notification is passed through a flash message in the session, defined on the server side. It's great I think, despite it is not easily customizable (cf below).

• For delete (Ajax request), the message is defined on the client side (in the javascript part of list.blade.php).

So, this is not consistent and not intuitive. Even for create and edit, it seems difficult to customize the message except I miss something. For these ones, I do this in my Controller, which is very ugly:

public function store(StoreRequest $request)
{
     [....]
     if (condition) {
            \Prologue\Alerts\Facades\Alert::flush();
            \Prologue\Alerts\Facades\Alert::warning(__('The item bas been submitted for review'))->flash();
     }
    [...]
}

For the delete, currently I modified list.blade.php but this is ugly too.

A way to rationalize all this stuff might be to always use flash messages defined on the server side. Proposed approach:

1. In alert.blade.php (Crud base), include the existing code in a function and call this function, so it can be called from another location in the code. In addition, the icon should be also a variable (currently set to false).

2. Define in CrudController a function "setNotificationMessage($type, $text, $icon)". If this function is called, a custom message is set and the default messages are not set (or removed).

3. In list.blade.php, call to the function defined in alert.blade.php to display all flash messages.

4. In the destroy function of CrudController, set the message through a flash Alert::success or Alert::error

This will offer the possibility to the users to use store, update and destroy to define their own messages (with the icon!) as they want.

Thoughts?

[tabacitu]

Hi @jrpub ,

I agree with needing a consistent way of defining notifications on CRUD operations, and that being PHP (using flash messages or not).

I think I've figured out a simpler approach, though. Let me know if it fits your use case, like I think:

1. The javascript delete logic has been moved in CRUD 3.4 from list.blade.php to buttons/delete.blade.php:

@if ($crud->hasAccess('delete'))
    <a href="javascript:void(0)" onclick="deleteEntry(this)" data-route="{{ url($crud->route.'/'.$entry->getKey()) }}" class="btn btn-xs btn-default" data-button-type="delete"><i class="fa fa-trash"></i> {{ trans('backpack::crud.delete') }}</a>
@endif

<script>
    if (typeof deleteEntry != 'function') {
      $("[data-button-type=delete]").unbind('click');

      function deleteEntry(button) {
          // ask for confirmation before deleting an item
          // e.preventDefault();
          var button = $(button);
          var route = button.attr('data-route');
          var row = $("#crudTable a[data-route='"+route+"']").parentsUntil('tr').parent();

          if (confirm("{{ trans('backpack::crud.delete_confirm') }}") == true) {
              $.ajax({
                  url: route,
                  type: 'DELETE',
                  success: function(result) {
                      // Show an alert with the result
                      new PNotify({
                          title: "{{ trans('backpack::crud.delete_confirmation_title') }}",
                          text: "{{ trans('backpack::crud.delete_confirmation_message') }}",
                          type: "success"
                      });

                      // Hide the modal, if any
                      $('.modal').modal('hide');

                      // Remove the row from the datatable
                      row.remove();
                  },
                  error: function(result) {
                      // Show an alert with the result
                      new PNotify({
                          title: "{{ trans('backpack::crud.delete_confirmation_not_title') }}",
                          text: "{{ trans('backpack::crud.delete_confirmation_not_message') }}",
                          type: "warning"
                      });
                  }
              });
          } else {
              // Show an alert telling the user we don't know what went wrong
              new PNotify({
                  title: "{{ trans('backpack::crud.delete_confirmation_not_deleted_title') }}",
                  text: "{{ trans('backpack::crud.delete_confirmation_not_deleted_message') }}",
                  type: "info"
              });
          }
      }
    }

    // make it so that the function above is run after each DataTable draw event
    // crud.addFunctionToDataTablesDrawEventQueue('deleteEntry');
</script>

1. Keep in mind this is how the delete method looks in the controller:

/**
     * Delete a row from the database.
     *
     * @param  int $id The id of the item to be deleted.
     *
     * @return bool True if the item was deleted.
     *
     * @throws \Illuminate\Database\Eloquent\ModelNotFoundException if the model was not found.
     *
     * TODO: should this delete items with relations to it too?
     */
    public function delete($id)
    {
        return (string) $this->model->findOrFail($id)->delete();
    }

1. I think a quicker way to achieve this would be to make the DELETE method return something else instead of true/false. Like a JSON:

[
  'success' => true,
  'message' => [
     'title' => 'lorem ipsum',
     'text' => 'lorem ipsum',
     'type' => 'danger',
     'icon' => 'lorem ipsum',
  ]
]

1. You'd then be able to easily customize that message in the Controller.
2. Of course, a fallback message would still be in the blade file, if the request is not processed correctly (AJAX fails).

What do you think?

[jrpub]

It sounds good.
I forgot to update to 3.4 what a shame. Doing it now... Thanks for the reminder!

[tabacitu]

Closing this, as it's been fixed in 3.4 - the javascript in the delete button shows an error message if the operation wasn't successful.

[tabacitu]

Oups - the issue was that we'd want to _customize_ that error message. Reopening it and marking as to-do for 4.0.

[tabacitu]

Good news everyone! We'll finally have this in 4.1 - if you're interested please follow the PR here https://github.com/Laravel-Backpack/CRUD/pull/2393