Jump to content

[Solved]Suggestion for an Hook improvement


3fingers
 Share

Recommended Posts

Hello guys,

I've written an Hook that it's working perfectly right now but I'd like to have some suggestion from you for a possible improvement.

Background:

- In a template "articolo" I've got a checkbox  "in_evidenza".
- When a user check "in_evidenza" on a page I want my Hook to set the value of all the other "in_evidenza" checkboxes (inside the other "articolo" pages) to 0.

Here is my Hook:

wire()->addHookAfter('Pages::saved(template=articolo)', function($event) {
    $pages = $event->object;
    $page = $event->arguments[0];
    $news = $pages->find("in_evidenza=1");
    $in_evidenza = $page->in_evidenza;
    if($in_evidenza == 1) {
        foreach($news as $new) {
            $new->setAndSave('in_evidenza',0);
        }
    }
    $page->setAndSave('in_evidenza',1);
});

Right now, as I mentioned, it is working but I've noticed that if i remove the condition (see below) the hooks sometimes works and sometimes the "Fatal Error: memory exhausted" error appears.

// Stripped the (template=articolo) selector
wire()->addHookAfter('Pages::saved', function($event) {
...
});

So I'm here to ask if you know a safer and more efficient way to perform my first Hook, even though right now it's working perfectly (I want to avoid some other memory errors in the future).

Thanks!

Link to comment
Share on other sites

Thanks @psy!

Right now my Hook looks like this (I've changed something more from my previous post):

/*
-- Changed: from Pages:saved to Pages::saveReady.
Doing that I can avoid to setAndSave the "in_evidenza" checkbox on the current page.
-- Changed: from "find" to "findMany".
Even though I actually got few pages to search into, they could grow in the future.
I didn't noticed speed improvements, but seems ok.
*/

wire()->addHookAfter('Pages::saveReady(template=articolo)', function($event) {
    $pages = $event->object;
    $page = $event->arguments[0];
    $news = $pages->findMany("in_evidenza=1");
    $in_evidenza = $page->in_evidenza;
    if($in_evidenza == 1) {
        foreach($news as $new) {
            $new->setAndSave('in_evidenza',0);
        }
    }
});

I'm still open for suggestions!

  • Like 1
Link to comment
Share on other sites

You're doing a needless check, because your query already returned only pages with `in_evidenza=1`

 $in_evidenza = $page->in_evidenza;
    if($in_evidenza == 1) {

Your code could be simplified (untested):

wire()->addHookAfter('Pages::saveReady(template=articolo)', function($event) {
    $pages = $event->object; // this should also be not needed
    foreach($pages->findMany("in_evidenza=1") as $new) {
   	 $new->setAndSave('in_evidenza',0);
    }
});

 

  • Like 1
  • Thanks 1
Link to comment
Share on other sites

Here is my final version, I post it here in case somebody needs it for a similar situation. Read the comment in code.

// --Added remove method to the query.
I did this because, otherwise, when a user make a 2nd edit to the current page (after the initial save) the "in_evidenza" checkbox would then be reset to 0.

wire()->addHookAfter('Pages::saveReady(template=articolo)', function($event) {
    $pages = $event->object;
    $page = $event->arguments[0];
    if($page->in_evidenza == 1) {
        foreach($pages->findMany("in_evidenza=1")->remove($page) as $new) {
            $new->setAndSave('in_evidenza',0);
        }
    }
});

Could any forum moderator mark this thread as solved? Thanks!

Link to comment
Share on other sites

  • 3fingers changed the title to [Solved]Suggestion for an Hook improvement

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

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...