24 replies to this topic

[Nico Knoll]

Here it is: The delete link in the page list

Download:
https://github.com/N...ocessPageDelete

[ryan]

Nice job Nico!

Just a couple suggestions:

Get rid of this line in the code below, you don't need it. It's also not very safe because it will load any page named trash in the site, so if someone has another page named trash anywhere in the site, your API call might load it instead of the one you want. That could be solved by prepending a '/' to the beginning, but you won't even need this line so just delete it:

$trash = $this->pages->get('name=trash'); // delete this

You are currently doing this:

$page->parent = $trash;

Instead, it's preferable to use the API function (which is why you won't need $trash anymore):

$this->pages->trash($page);

For security, in your execute() function, you should add this immediately after retrieving the $page:

if(!$page->deleteable()) throw new WirePermissionException("You don't have access to delete that page");

You are already performing this check in your function that adds the action to PageList, which is good. But it's not providing any security unless you also perform the permission check in your execute function().

[transpix]

Finally found time to add this to my site!

After making the changes suggested I was getting the following error after clicking delete;

Can't save page 1323: /trash/1323_test_page/: Chosen parent '/trash/' already has a page named '1323_test_page'

The page was still moved to trash though. I removed the following line and that fixed the error;

$page->save();

I didn't try the module before making the changes, but I guess the change to;

$this->pages->trash($page);

calls the save page method anyway?

[transpix]

I just made one other little tweak so that the pages tree opens up at the level of the page you just deleted.

After the page deleted exception;

if(!$page->deleteable()) throw new WirePermissionException("You don't have access to delete that page");

I added the following to get the parent id;

$parent_id = $page->parent_id;

...and then altered the redirect to;

$this->session->redirect("../?open={$parent_id}");

[ryan]

Nico, Transpix -- it's correct that you don't need the save() if you are already using $pages->trash($page). that's because the trash() function already performs the save for you.

[onjegolders]

Thanks for this, is there anyway of adding a JS popup to confirm delete?

[Nico Knoll]

New version is uploaded.

I added the popup confirmation and integrated transpix's improvements.

[onjegolders]

Thanks Nico! Will try it out in the morning

[onjegolders]

My users are having problems deleting. It just returns them to main admin page and says that they are logged in? The traditional method of edit then delete works for them however. Any ideas? Thanks Nico

[Nico Knoll]

Which Browser, which OS (Mac/Windows/Linux)? Is Javascript activated?

[onjegolders]

Sorry, tried on OSX and Windows, both with JS.
Can I give you any more info?
It works great for me as a superuser.

[Nico Knoll]

So it's only not working for not superusers? Which permissions are the normal users having?

[onjegolders]

No sorry, it's working for superusers but editors (which is the only other group I have) cannot delete, but they can delete from within the entry (edit->delete)

[Nico Knoll]

Yeah, but could you look at "permissions" -> "roles" which rights the user groups are exactly have?

[apeisa]

Nico: not tested, but probably just adding

'permission' => "page-delete"

To your getModuleInfo might do the job.

[Nico Knoll]

@apeisa OK, I added it and uploaded the new version. Maybe it'll work

[Marty Walker]

What a great time saver. Thanks Nico!

[Marty Walker]

Hi Nico,

Is there any reason why the javascript below is being inserted into my templates?

Regards
Marty

<script type="text/javascript">
  $(document).ready(function() {
   $("li.PageListActiondelete a").live("click", function(el){
    if(!confirm("Are you sure?")) { return false; }
   });
  });
  </script>

[Nico Knoll]

Well, I guess I forgot to insert a command to check if you are in the backend. Will add it today

[Soma]

Well, I guess I forgot to insert a command to check if you are in the backend. Will add it today

Have you added the check already? Creating modules means also maintaining them, continue to fix stuff.

Edit: I also would suggest to name the module "PageListActionDelete". So it's a little more clear its extending the actions on the page list.
Also before I forget it, would be nice to have it translateable too.