Jump to content

Putting page hooks in Page Classes


MarkE
 Share

Recommended Posts

I'm sure I read something about this recently, but I can't find it ... (is there a way of bookmarking forum posts?)

I'm trying to move my page hooks out of ready.php into Page Classes, which I think are tremendous and use extensively. But what is the best way to do it? ready() doesn't trigger automatically. I tried 

    public function ___loaded() {
        $this->ready();
    }

(thanks to @Robin S) and this does trigger it. However, if I do, say 

    public function ready() {
        $this->addHookAfter('Pages::save', $this, 'afterSave');
        $this->addHookBefore('Pages::save', $this, 'beforeSave');
    }

then the hook methods get triggered by any page save, not just of instances of this class. I solved this by adding a breakout at the start of the method:

    protected function beforeSave(HookEvent $event) {
        $p = $event->arguments(0);
        if ($this != $p) return;
......

but this seems a bit inelegant. Any better ideas?

Link to comment
Share on other sites

4 hours ago, MarkE said:

public function ___loaded() {

I have just posted about my findings regarding ___loaded(), see: https://processwire.com/talk/topic/25343-putting-page-hooks-in-page-classes/ Note that I used the latest dev (3.0.174) to do my testings.

4 hours ago, MarkE said:

then the hook methods get triggered by any page save, not just of instances of this class.

The issue is that same I described in my post, I think. ___loaded() gets called for any pages when using "direct subclasses" whereas going further down the class hierarchy, this is no longer the case.

Regarding hooks, I found that applying them in the constructor works as expected, at least the following way:

//direct subclass
class DefaultPage extends Page {}

plus:

//subclass of direct subclass
class ArticlesPage extends DefaultPage {

	public function __construct(Template $tpl = null) {
		parent::__construct($tpl);
		$this->wire->addHookAfter("Pages::saveReady", $this, "onSaveReady");
	}

	//this one gets called as expexted upon saving the "Articles" page but not when saving others
	public function onSaveReady(HookEvent $event) {
		$page = $event->arguments(0);
		bd($page); //Tracy Debugger is required for this one :)
	}
}

 

Edited by szabesz
typo fix
Link to comment
Share on other sites

    Snap!!

33 minutes ago, szabesz said:

think. ___loaded() gets called for any pages when using "direct subclasses" whereas going further down the class hierarchy, this is no longer the case.

I find it is the case for both direct and subclasses

33 minutes ago, szabesz said:

Regarding hooks, I found that applying them in the constructor works

Nice plan - quite neat and maybe better than using ___loaded() - but I still find the called method operates on all pages, so I need to filter with

    protected function beforeSave(HookEvent $event) {
        $p = $event->arguments(0);
        if ($this != $p) return;

It may be that my use case is rather different and that, in other circumstances, your solution works. In my case a button on the page executes a method which saves pages of a different class, but these still get caught by the hook.

EDIT: I find that @bernhard's solution has the same issue in my use case. The problem does not 'appear' to exist when using ready.php, because you naturally do $p = $event->arguments(0); there anyway and test for template before passing to the class method.

Link to comment
Share on other sites

@MarkE Have you perhaps done anything "not so documented" besides simply placing your class files in /site/classes/?

For example, in an effort to relocate the class files, I tried to do this:

$classLoader->addSuffix('Page', __DIR__ . "/templates/abode/{$template}");

as suggested by @teppo over here but doing so somehow results in the behavior you describe, that is: there is no difference between direct subclasses and subclasses of those in this regard we are discussing right now.

Can you please provide more details about what and how you are trying to achieve so that I can also test that?

 

Link to comment
Share on other sites

3 hours ago, szabesz said:

simply placing your class files in /site/classes/

That's where they are. I am also trying 

$this->addHookAfter('Pages::save(template=TemplateName)', $this, 'afterSave');

but can't get it to work at the moment.

Link to comment
Share on other sites

Please see my post here

 

9 hours ago, MarkE said:

Nice plan - quite neat and maybe better than using ___loaded() - but I still find the called method operates on all pages, so I need to filter with


    protected function beforeSave(HookEvent $event) {
        $p = $event->arguments(0);
        if ($this != $p) return;

That's correct behaviour. You are attaching the hook in your custom page class, but the hook you are attaching belongs to "Pages" and not to the custom page class.

If you attach a Pages::saveReady hook, it will always fire on Pages::saveReady having the saved page as arguments(0) and therefore you need to check if that page is your custom pageclass or not.

BTW I'd recommend doing this instead of $this != $p

$page = $event->arguments(0);
if(!$page instanceof self) return;

I've had problems using $page != $this when some properties of my page have changed and therefore the check returned false even though it was an instance of my custom pageclass. "instanceof" should always work as expected. You could also just check for $page->template if you prefer...

 

  • Thanks 1
Link to comment
Share on other sites

12 minutes ago, MarkE said:

That's where they are. I am also trying 


$this->addHookAfter('Pages::save(template=TemplateName)', $this, 'afterSave');

but can't get it to work at the moment.

I already showed an example here https://processwire.com/talk/topic/21212-rockmigrations-easy-migrations-from-devstaging-to-live-server/?do=findComment&comment=212496 and here

https://processwire.com/talk/topic/25342-custom-classes-for-page-objects-the-discussion/?do=findComment&comment=212563

HiHxiGI.png

of course you can use $tpl = "template=yourtemplate" instead of using the class constant.

 

  • Like 1
Link to comment
Share on other sites

Are you sure you want to use Pages::save() ?

This does NOT mean that the page has been saved. For this we have the Pages::saved() hook! You are hooking into saving a single page field value, eg $page->save('myfield', 'myvalue');

Link to comment
Share on other sites

1 hour ago, bernhard said:

BTW I'd recommend doing this instead of $this != $p

There is a subtle but important difference between the above and your suggestion. Actually in this case, I needed $this != $p because I did not want the hook to run on any page other than the current instance. Generally, to filter the page classes, your suggestion is the right one. I am now doing the filtering with (template=...) as per my earlier post, so the filter in the method is no longer needed. However, as I have now discovered, for my use case, I do still need to check that the save being hooked is of the current instance - which can either be done by $this != $p or by using $p thoughout in the hook method (not sure which is better...) .

1 hour ago, bernhard said:

Are you sure you want to use Pages::save() ?

Good question! I am using the (before) hook to validate fields and so Pages::save() is right, I think.

Link to comment
Share on other sites

  • 1 year later...

@thetuningspoon magicpages are super cool ? the only drawback is that they come with a small performance penalty as I need to load all available templates once on init and then attach the hooks or magic methods on applicable templates: https://github.com/baumrock/RockMigrations/blob/87541c899901f773bf62068a639f070f48a85453/MagicPages.module.php#L43-L51

The init() and ready() method (and the magic methods) are only called once for every template because I create one runtime page for each template on ProcessWire::init

The magic methods on the other hand only trigger for the correct pageclass because I have an early exit in the hook if the pageclass does not match: https://github.com/baumrock/RockMigrations/blob/87541c899901f773bf62068a639f070f48a85453/MagicPages.module.php#L113-L119

You might also like this:

 

  • Like 1
Link to comment
Share on other sites

  • 1 year later...

Has there been an updated, more formal, non-MagicPage approach to this yet?

Is there anything wrong with doing this so at least the hook is "closer" to the page class?

<?php namespace ProcessWire;

// this is /site/classes/OrderPage.php
// hook above class so it's "closer"

wire()->addHookAfter("Pages::saved(template=order)", function(HookEvent $event) {
  $order = $event->arguments('page');
  // hook code
});

class OrderPage extends Page {
  // ...
}

Related GitHub issue since I don't think it's linked in any of the replies above:
https://github.com/processwire/processwire-requests/issues/456

  • Like 3
Link to comment
Share on other sites

5 hours ago, Jonathan Lahijani said:

Is there anything wrong with doing this so at least the hook is "closer" to the page class?

This does introduce a side-effect to the class file. While it's not "wrong", it is discouraged, and will cause some code checkers to complain about the code. For example, if you are running codesniffer with one of the PSR standards, this would probably be flagged up (not checked.)

  • Like 2
Link to comment
Share on other sites

23 minutes ago, netcarver said:

This does introduce a side-effect to the class file. While it's not necessarily "wrong", it is discouraged and will cause some code checkers to complain about the code. For example, if you are running codesniffer with one of the PSR standards, this would probably be flagged up.

Interesting idea @Jonathan Lahijani and thx for the input @netcarver! Is something like this also a problem? https://github.com/baumrock/RockFrontend/blob/107af5b51930d0589ecc0a882e46372b640eb6f9/RockFrontend.module.php#L26-L29

  • Like 1
Link to comment
Share on other sites

@bernhard No, I think that's just a function declaration before a class declaration.  If in doubt, you can run phpcs on your code.  If you are only looking for side-effect warnings just grep your output for "effects"...

phpcs --standard=PSR12 file.php | grep "effects"

Run it without the grep for the full PSR12 ruleset...

phpcs --standard=PSR12 file.php
  • Like 1
  • Thanks 1
Link to comment
Share on other sites

@Jonathan Lahijani Please don't interpret what I wrote in reply to your post as negative - it's not meant to be. That may well be a fine solution for this. I appreciate knowing about the option, and having the link to the github issue you posted is a major bonus - I missed that one until now.

  • Like 3
Link to comment
Share on other sites

  • 4 weeks later...
On 5/14/2024 at 3:42 AM, Jonathan Lahijani said:

Has there been an updated, more formal, non-MagicPage approach to this yet?

An approach I sometimes use is the following.  It seems to work quite well and is fairly clean:

  1. Put all your class-specific hooks in your page class, let's say in a ready() function (can be named anything of course, but ready() is a good convention for me).
  2. In /site/ready.php, just instantiate a single instance of the page class and call its ready() method.

So, if we have let's say a NewsarticlePage class and we want to keep all news article related hooks together,  we might have something like this in the page class:

<?php

namespace ProcessWire;

class NewsarticlePage extends DefaultPage
{
	public function ready()
    {
        $this->addHook('ProcessPageEdit::buildForm', $this, 'addImportOptions');
        $this->addHookAfter("Pages::saved(template=newsarticle)", $this, 'processAfterSaveActions');
        $this->addHookAfter("Process::execute", $this, 'processAfterExecuteActions');
    }

And in site/ready.php:

<?php

namespace ProcessWire;

//required to trigger ready hooks in custom page classes:
(new NewsarticlePage())->ready();
(new SomethingElsePage())->ready();
//etc...

You can instantiate one empty page object for each Page class you need to run hooks for.  There's some overhead, but not much, and it keeps your site/ready.php clean and your hooks where they make sense.  Of course you still have to make sure the hooks only target the desired pages/templates.

Inspired by some of @bernhard's earlier posts on the subject.

 

  • Like 2
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

  • Recently Browsing   0 members

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