Jump to content
3fingers

[Solved]Suggestion for an Hook improvement

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!

Share this post


Link to post
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

Share this post


Link to post
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

Share this post


Link to post
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!

Share this post


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

 

  • Like 2

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


  • Recently Browsing   0 members

    No registered users viewing this page.

×
×
  • Create New...