Jump to content
feniks502

Hook runs twice

Recommended Posts

Hello!

The only hook into 'trash' in a module runs twice, by itself, as it seems.

The following code outputs:

  • Session: Hooked 1 times on abcd
  • Session: Hooked 2 times on abcd

on the admin side when one page is deleted from it's delete tab.

And it's no matter 'addHookBefore', 'addHookaAfter' or 'addHook' method is used.

Why?!

private $i = 0;

public function init() {

	$this->pages->addHookBefore('trash', $this, 'test');

}

public function test($event) {

	$page = $event->arguments('page');

	$this->i++;
	$this->message("Hooked {$this->i} times on {$page->title}");

}

Share this post


Link to post
Share on other sites

Hi,

I could reproduce this in current dev 3.0.16. .   Pages::trash seems to be executed twice.
Is this still up to date for you? Maybe anyone else has a hint on what is wrong with it?

Greetings

Share this post


Link to post
Share on other sites

Hi,

I could reproduce this in current dev 3.0.16. .   Pages::trash seems to be executed twice.

Is this still up to date for you? Maybe anyone else has a hint on what is wrong with it?

Greetings

I had found nothing about this.

Just updated my installation to dev 3.0.17 and still have two messages instead of one.

It is critical for me, because  I have write current amount of 'Pages' within the structure like 'Parent(1) -> Parent(1.1) + Parent(1.2) +Parent(1.3) , ... -> Pages' to the 'Parent(1)' each time it is changed, so I don't know any other way, than hook into 'save', 'trash', 'delete', 'move' methods.

Particularly, concerning the 'trash' or 'delete' methods, I have to hook into them before execution to find '$page->parent->parent', to which data will be written (as you see after that it is impossible), but with 'trash' method my 'beforeHook' saves '$page->parent->parent' to a module class property correctly, and then it is get overwritten immediately, so it looks like 'addHookBefore' adds hook before and after.

Share this post


Link to post
Share on other sites

Hi thanks for that hint. I can only try it out later.

For feniks, this might not help as I understood, as he needs the page before it is trashed, but maybe it could work if you change your hook method (test above) with a check if the page is trashed? If there are really two different calls like Soma suggested, it should be one before actually trashed and in the second call of the modified page before save(?).
Maybe you get to try it out before me:

$page = $event->arguments('page');
if($page->isTrash()) return; //should only happen on the second call(?)

However I did not fully understand your use-case fully. Depending on what you want, it might be much easier to just count the children where you need them in Parent(1) or if you know the parent(s) you want the count for, you could also do it easier I guess:

// e.g. just:
$page->numChildren;
// in the template, or outside e.g:
$pages->count("has_parent=/locationOf/Parent");

Hope this helps.

Share this post


Link to post
Share on other sites

I ran into this same issue with triggers 'trash' and 'restore'. I was trying to do stuff before page is trashed/restored. I saw weird behavior but it took a good while until I understood triggers were being triggered multiple times per request.

My use case: I need to communicate with REST API when page changes. Running triggers multiple times is an issue as it leads to unexpected results. I had to go around the issue with class properties, but I don't think that should be the case. I assumed addHookBefore('Pages::restore'...) would be run once.

Share this post


Link to post
Share on other sites

I know it's a long time since the last post, but this still seems to be an issue in 3.0.123. It's easily avoided by setting a flag to prevent repeats (see below), but is it a bug or a side-effect of some intended behaviour?

wire()->addHookBefore('Pages::trash', function ($event) {
    $p = $event->arguments(0);
    if ($this->skip != $p->id) {
        $this->skip = $p->id;

 

Share this post


Link to post
Share on other sites

For anyone wanting to trace how it is that Pages::trash is called twice...

Pages::trash (first call) calls PagesTrash::trash, and when the "save" argument is true (as it is when trashing via the admin) then Pages:save is called, which calls PagesEditor::save.

And if that saved page is in the trash then Pages::trash is called (second call) with the "save" argument false.

As to whether this second Pages::trash call is necessary and correct, I don't know.

Best thing is to hook Pages::trashed as suggested above - this method only fires if the page is successfully trashed, which is probably what is wanted in most cases.

  • Like 3

Share this post


Link to post
Share on other sites
1 hour ago, Robin S said:

(second call) with the "save" argument false.

Aha! So a better way to avoid the hook being called twice would be to test for the 'save' argument.

1 hour ago, Robin S said:

Best thing is to hook Pages::trashed as suggested above

In my case I can't use the trashed hook as I need to access the parent of the page being trashed, which is too late if it has been trashed.

Share this post


Link to post
Share on other sites
26 minutes ago, MarkE said:

test for the 'save' argument.

Except that, in the 'before' hook, the test is is_null() because the default 'true' value has not been set. The second time, the argument will be 'false', which is not null 😉 

Share this post


Link to post
Share on other sites
48 minutes ago, MarkE said:

In my case I can't use the trashed hook as I need to access the parent of the page being trashed, which is too late if it has been trashed.

Yeah, makes me wonder if it would have been better if the Pages::trashed method was called immediately before saving the trashed page rather than after. Maybe Ryan has a good reason for doing it that way.

I still think it's better to hook after Pages::trashed if you want to know for sure which pages are trashed because when hooking Pages::trash there are still instances where the trashing can fail. For example, there might be another Pages::trash hook in a module that deliberately prevents trashing of particular pages.

If you hook Pages::trashed you can parse the parent page ID (and some other info) from the name of the page in the trash:

$pages->addHookAfter('trashed', function(HookEvent $event) {
	$page = $event->arguments(0);
	$pages = $event->object;
	/* @var PagesTrash $trasher */
	$trasher = $pages->trasher();
	$name_info = $trasher->parseTrashPageName($page->name);
	if(!empty($name_info['parent_id'])) {
		$parent = $pages($name_info['parent_id']);
		// ...
	}
});

 

  • Like 1

Share this post


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

  • Recently Browsing   0 members

    No registered users viewing this page.

  • Similar Content

    • By Guy Incognito
      I've created a simple sports league fixture generator in a template called 'League'. Teams are added as page references then a fixture list is created as a ProFields table by hooking page save to add new rows to the table.
      The bit I need help with is that I'm trying to check a fixture doesn't already exist before adding it to the table (e.g. if a new team is added to the league). I'm trying to do this with a PW selector to filter the fixtures table and check whether Team A vs Team B already exists in the table. Then on the next line checking no fixture was found by using count().
      However as soon as I add the selector the script no longer adds any rows to the table. If I take it out, it all works fine (albeit with duplicate fixtures each time the page is saved). I've also tested the selector in a page template and it filters as expected. It's late here (UK)... I'm probably doing something stupid! Any ideas?
      <?php //Hook page save to generate league fixture lists $wire->addHookAfter("Pages::saved(template=league)", function ($event) { //Get which page has been saved $page = $event->arguments(0); $noFixturesAdded = 0; //For each team in league cycle through and add home fixtures foreach ($page->teams_in_league as $teamA) { foreach ($page->teams_in_league as $teamB) { //Check if fixture already exists $existingFixtures = $page->fixtures("team_a=$teamA,team_b=$teamB"); //Check team A is not the same as team B as you can't play yourself //Then add row to fixture table if ($teamB != $teamA && $existingFixtures->count() < 1 ) { $fixture = $page->fixtures->makeBlankItem(); $fixture->team_a = $teamA->id; $fixture->team_b = $teamB->id; $page->fixtures->add($fixture); $noFixturesAdded ++; } } } //Save updates to table $page->save('fixtures'); $message = "League saved. $noFixturesAdded new fixtures were added"; $this->message($message); });  
    • By Hardoman
      Hello community,
      we have a website running version 3.0.118. The owner would like to have a watermark merged to the images, that are being uploaded in the CKEditor as a requirement.
      Image upload besides the CKEditor within galleries and single images works as a charm already. We also use croppable image 3 there. (PIM2)
      To realize this requirement, I thought of using a hook in the admin area. So, I read a lot in our forums and tested this by adding a hook into the ready.php file.
      $this->addHookAfter('InputfieldFile::fileAdded',function(HookEvent$event){ wire('log')->save('test','Image upload works'); ... The log entry is being created correctly. But when I try to use the pim/watermark-function like in a template, he cannot find the watermark-image anymore. Furthermore, when I try to get the page-id, it does not seem to be accessible, because the application does not seem to know how to reference it, or I dont know the right way to do so…
      So my questions are:
       
      Is this the right attempt at all or will there be another, better workaround? It seems, I cannot access the page object (of the content page) within this scope or file but I would need it to save the processed image inside the right files/id folder Would it be better to place the hook into the admin-template? (or admin.php)
        Thanks for any hints in advance. 🙂
    • By martind
      hi,
      is it possible to change field parent_id from InputfieldPageListSelect to InputfieldPageAutocomplet by addHookAfter('ProcessPageEdit::buildFormContent')?
      thanks,
      martin
       
    • By Noel Boss
      👋 PW Pros…
      I have some hooks that I need to bind at the init phase (or even __construct) and I was wondering, and I couldn't find a good and simple way to determine if I'm in the admin. Would be nice if there is a reliable short option to do so, but I can't seem to find one… Is there a coherent way to tell this no matter where I am?
      Right now, I use the following method inside one of my modules:
      public function isAdmin($page = null) { if ( strpos($this->input->url, $this->urls->admin) !== false || $this->process instanceof ProcessPageList || $this->process instanceof ProcessPageEdit || ($page instanceof Page && $page->rootParent->id == $this->config->adminRootPageID) ) { return true; } return false; } @ryan wouldn't it be nice to have something like wire()->isAdmin(); like wire()->user->isLoggedin(); to tell if we are in admin – very early on (probably even in __construct() phase of modules?
    • By Macaco
      It's a bilingual site. There are two pages: "Artists" and "Events" each with a "Page Reference" field connecting each other.
      - Artists has a field where one can choose events available.
      - Events has a field where you can either choose artists available or create new ones.
      The problems: 
      - When I create an "Artist" page and select events from the list, it doesn't update the collection of participating artists on the "Event" page.
      - When I create an artist from the "Event" page. The field 'artist page > settings > language' is not "Active" for the second language.  When the artist page is created manually,"Active" is on by default.
      I know this all have to do with hooks, but I'm don't fully understand the logics.
×
×
  • Create New...