[Solved]Suggestion for an Hook improvement

[3fingers]

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!

[psy]

@3fingers rather than 'find', 'findMany'. 'find' loads all found pages in memory. Another alternative maybe @bernhard's RockFinder module?

[3fingers]

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!

[dotnetic]

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

 

[3fingers]

Your improved version works flawlessly, and yes, $pages->$event->object; is needed (error otherwise).

Thanks @jens.martsch!

[3fingers]

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!

[psy]

@3fingers edit your original post title  with [solved] at the start ? 

[bernhard]

wire()->addHookAfter('Pages::saveReady(template=articolo,in_evidenza=1)', function($event) {
	$reset = $this->pages->find("in_evidenza=1");
	foreach($reset as $p) $p->setAndSave('in_evidenza', 0);
});