Jump to content

ProcessPageDelete


Nico Knoll
 Share

Recommended Posts

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().

Link to comment
Share on other sites

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?

Link to comment
Share on other sites

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}");
Link to comment
Share on other sites

  • 2 months later...
  • 3 weeks later...

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>
Link to comment
Share on other sites

  • 2 months later...

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. :D

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. ;)

Link to comment
Share on other sites

  • 2 months later...
  • 5 months later...

Hi, just tested this module in demo mode $config->demo = true;

In demo mode is possible delete(move to trash) pages.

Nico please make update to deny deleting pages in demo mode.

Thanks

  • Like 1
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...