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

Event cancels delete but backpack reports item deleted #1829

Closed
basdog22 opened this issue Mar 6, 2019 · 4 comments
Closed

Event cancels delete but backpack reports item deleted #1829

basdog22 opened this issue Mar 6, 2019 · 4 comments
Labels

Comments

@basdog22
Copy link

basdog22 commented Mar 6, 2019

Ok, I set up a crud controller and works as expected.

What I did:

For some reason, we need an observer on the crud model to check if an item can be deleted. So, i created one and if the item cannot be deleted i return false:

    {
        if(Dedicated::hasActiveFor($hostingPackage->id)){
            \Alert::warning(trans('app.services_active_cant_delete'))->flash();
            return false;
        }
    }

which does the job as expected.

What I expected to happen:

Backpack to show a notification that the item cannot be deleted... (services_active_cant_delete)

What happened:

Backpack shows a notification that the item was deleted and when i reload the page i get my warning notification.

Backpack, Laravel, PHP, DB version:

Laravel 5.7, PHP 7.2

@welcome
Copy link

welcome bot commented Mar 6, 2019

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication mediums:

  • Bug Reports, Feature Requests - Github Issues (here);
  • Quick help (How do I do X) - Gitter Chatroom;
  • Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the backpack-for-laravel tag;

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

--
Justin Case
The Backpack Robot

@basdog22
Copy link
Author

basdog22 commented Mar 7, 2019

Ok. I think i found it. The delete button view (delete.blade.php) file does not handle the result at all:

$.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 details row, if it is open
	                  if (row.hasClass("shown")) {
	                      row.next().remove();
	                  }
	                  // 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"
	                  });
	              }
	          });

I override the button now and i add my logic for handling the result

@Athuli7
Copy link
Contributor

Athuli7 commented Apr 1, 2019

@tabacitu, any take on this? This is one of the easiest ways to handle it without deviating from the Laravel way to things.

@basdog22, We should probably also have the option to show the reason inside the notification. Otherwise, the mistake will be unknown to the end user.
Eg, my financials CRUD, disables the delete option once the record is confirmed or if there aren't enough funds to reverse the record.

This type of logic can be implemented in the event, but how to show them? Throwing an exception and displaying the error on the notification is an option, but that just throws errors, which is not a great idea.

Alert::warning(trans('app.services_active_cant_delete'))->flash(); won't work because the messages are shown whent the page loads. Unlike the javascript pnotify implementation which throws immediately.

@tabacitu
Copy link
Member

tabacitu commented Apr 9, 2019

Hmm... you're right guys. Thanks for the heads-up. I've just pushed an update to the delete.blade.php button, so that when the even does NOT happen, a warning bubble shows up instead.

@Athuli7 indeed you can't reply with a custom message... not with the current API...

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

No branches or pull requests

3 participants