Sidebar Hooks don't work anymore

[yggdrasil]

I upgraded to the latest version 7.10.2 and for my surprise even the example hooks that come from the official WHMCS site now break WHMCS.

Example:

https://docs.whmcs.com/Client_Area_Sidebars_Cheatsheet

<?php

use WHMCS\View\Menu\Item as MenuItem;

add_hook('ClientAreaPrimarySidebar', 1, function(MenuItem $primarySidebar)
{
    $primarySidebar->getChild('My Account')
        ->getChild('Billing Information')
        ->setUri('https://www.example.com/billingInfo');
});

Results in:

Oops!

Something went wrong and we couldn't process your request.

Please go back to the previous page and try again.

I noticed this because some of my sidebar hooks worked in some pages and not others and then I started to test directly with an example from WHMCS and its causing a PHP errors in the WHMCS application related to getChild

WHMCS\\Utility\\SafeInclude::{closure}(Object(WHMCS\\View\\Menu\\Item))

Edited July 11, 2020 by yggdrasil

[yggdrasil]

The problem seems to be related to GetChild in the hook. When you are in a page that it cannot find that menu, it breaks.

Edited July 11, 2020 by yggdrasil

[brian!]

8 hours ago, yggdrasil said:

I upgraded to the latest version 7.10.2 and for my surprise even the example hooks that come from the official WHMCS site now break WHMCS.

you were surprised?? from the launch of Six, some of the hooks on the docs pages have been wrong... including the above (which is an abomination)... hell, the explanatory text around it doesn't even tally with the content of the actual hook. 🙄

8 hours ago, yggdrasil said:

I noticed this because some of my sidebar hooks worked in some pages and not others and then I started to test directly with an example from WHMCS and its causing a PHP errors in the WHMCS application related to getChild

never use examples from the docs - I wouldn't trust them to work... you'll be better off using the hooks posted in these forums as actual working examples.

the problem with the above hook is that it breaks the absolute golden rule of sidebar/navbar hooking - in that, if you are going to modify something, check that it even exists before trying to make any changes to it - otherwise, you'll get an error and that will look terrible to the customer if it's visible in the client area.

for example, the above hook would work fine if 'Billing Information' exists under 'My Account, but as soon as you go to a page where MyAccount sidebar doesn't exist (or even BI within it), then you'll get an Oops - all because the hook isn't checking whether it exists or not.... some of the other hooks on that page, do at least make a check first, but not the first one.

[yggdrasil]

Yes, I already noticed that. It only works on the page it's called (if the child menu is found) otherwise it crashes. I reported this a bug and WHMCS reply was that this fails because it's not finding the menu on the page as if crashing WHMCS with a hook using their own official example is the expected result.

Does this make sense? Not to me. The hook is supposed to work only on the page on which the menu exists?

1. It crashes WHMCS. This seems to be ok with them, not a bug.

2. It only works if the menu can be found. Crashes other pages.

Number two baffling to me. I'm supposed to add extra code and check on which page the hook is executed now, and only run if it can find that page and skip exceution on all others? Menu hooks run everywhere which is already bad in terms of performance, but now we are supposed to add extra code (which is completely missing in the documentation) to check if it's running on the right page? No other sidebar/navbar hook I have works like that because you assume navigation and sidebars are on almost all WHMCS pages, I can't possibly know what page the visitor will land on or what he will click next.

Look terrible to the customer? That is my other point. Hooks should fail gracefully, this is not causing an error on my WHMCS installation, it crashes it completely. It is basically as putting it offline as most pages will inaccessible and just display the internal error. You cannot continue after that. Its like hitting a 500 error page.

Edited July 11, 2020 by yggdrasil

[yggdrasil]

So what is the solution to this? Adding an else statement as such as?

use WHMCS\View\Menu\Item as MenuItem;

add_hook('ClientAreaPrimarySidebar', 1, function(MenuItem $primarySidebar)
{
    if (!is_null($primarySidebar->getChild('My Account'))) {
        $primarySidebar->getChild('My Account')
            ->getChild('Billing Information')
            ->setUri('https://www.example.com/billingInfo');

    }
});

Or is there a more efficient, simple and clean way?

Edited July 11, 2020 by yggdrasil
bug

[brian!]

20 minutes ago, yggdrasil said:

Yes, I already noticed that. It only works on the page it's called (if the child menu is found) otherwise it crashes. I reported this a bug and WHMCS reply was that this fails because it's not finding the menu on the page as if crashing WHMCS with a hook using their own official example is the expected result.

lol - the most generous interpretation I can think of is that the support guy didn't realise you were reporting an issue in the documentation and thought you had a real issue..... but as i've said before, oh boy! 🙄

22 minutes ago, yggdrasil said:

Does this make sense? Not to me.

nor me.

25 minutes ago, yggdrasil said:

The hook is supposed to work only on the page on which the menu exists?

as written, yes.

31 minutes ago, yggdrasil said:

1. It crashes WHMCS. This seems to be ok with them, not a bug.

it's not a bug in the software - it's poor documentation... and if you check archive.org, then you will find that same example hook code has been on that page for years.

35 minutes ago, yggdrasil said:

2. It only works if the menu can be found. Crashes other pages.

yep.

1 hour ago, yggdrasil said:

Number two baffling to me. I'm supposed to add extra code and check on which page the hook is executed now, and only run if it can find that page and skip execution on all others?

you can, but it's optional - you definitely have to check whether something exists before trying to modify it - regardless of whether you limit the hook to run on only certain pages.

1 hour ago, yggdrasil said:

Menu hooks run everywhere which is already bad in terms of performance, but now we are supposed to add extra code (which is completely missing in the documentation) to check if it's running on the right page?

generally speaking, you really only have to check whether what you're modifying exists before trying to modify it - if that check is coded correctly, then on pages where what you are trying to modify doesn't exist, the hook should do nothing... if it exists, then the hook does its thing.

if you are asking if there are ways to have menu hooks running only on certain pages/templates, then absolutely yes, you can do that in a hook  - i've posted many examples of how to get menu hooks running only on specific pages.

1 hour ago, yggdrasil said:

No other sidebar/navbar hook I have works like that because you assume navigation and sidebars are on almost all WHMCS pages, I can't possibly know what page the visitor will land on or what he will click next.

I think you're overcomplicating the problem here and panicking when there is no need to.

16 minutes ago, yggdrasil said:

So what is the solution to this?

oh i'm being subcontracted to fix the hooks in the documentation now! 😲

in many ways, it's a ludicrous hook example - because it's effectively trying to modify something that doesn't exist by default, and was therefore created with another custom hook - and if that's the case, you change the label in the original hook that creates it, not with a second hook - that way lies madness... and if you don't know what you're doing, an oops.

yes I know the documentation says the hook should be about something else - but it isn't, so i'm only dealing with the hook code, not what the description says the hook should contain.

24 minutes ago, yggdrasil said:

Adding an else statement as such as?

ok, for the sake of argument, let's pretend that the example hook is modifying the label of a child that was created by WHMCS by default, and we just want to change the label, with your modified hook....

• if "My Account" sidebar doesn't exist, the hook will do nothing.
• if "My Account" sidebar exists, but 'Billing Information' doesn't exist, the hook will cause an oops error.
• if both "My Account" and 'Billing Information' parent and child exist, the hook will change the label.

29 minutes ago, yggdrasil said:

Or is there a more efficient, simple and clean way?

one way to do it is already shown on that page....

if(!is_null($primarySidebar->getChild('My Account')) && !is_null($primarySidebar->getChild('My Account')->getChild('Billing Information'))){

which checks that both MA and BI exists before trying to modify BI.

another way would be getchildren, where you get the children of a sidebar/navbar etc in an array, loop through it and do what you need to do to the kids.... it can be a safer method to use because you remove the need to check whether something exists because you know that it does, but it can be more complicated for others to understand the coding.

[yggdrasil]

There is also a 404 link on that documentation that points a non-existing GitHub page. Not that it matters, but as you said, they probably don't look at the documentation or the examples for a while.

I understand some things are not very well documented. I would not say it's poor in general, WHMCS has better documentation that other competitors but if they are posting code examples which people use as the base for everything else, that should work. No excuses with that. Otherwise, people would try to figure forever what is wrong on their installations when nothing is wrong except the code is that is coming from WHMCS. I would rather have them remove the examples if they are broken vs having them posted incorrectly.

Instead of making fancy marketing videos about new features why isn't WHMCS investing on making proper video tutorials or documentation. If people have issues with the software because of the lack of information or decide to stop using it because they think something cannot be done when in fact it can, that means WHMCS is losing sales, and customers. As other developers said before, your documentation is part of your product. It's the biggest selling point. I actually think I purchased WHMCS because I started reading the documentation on what you could achieve with it. That was years back but this is something they seriously need to improve for developers.

With open source this is not that important as you can look at the code and debug it. With closed software like this, nobody knows what goes behind the scenes and there is no way to understand how WHMCS does something. For developers documentation is extremely vital. 

I understand you posted many examples here in the community but having to search every post, some that contains bugs as people posted before you post the right one, without mentioning that some are for older versions is annoying. I would rather have proper documentation and examples on the WHMCS docs page.

Edited July 11, 2020 by yggdrasil

[brian!]

13 hours ago, yggdrasil said:

There is also a 404 link on that documentation that points a non-existing GitHub page. Not that it matters, but as you said, they probably don't look at the documentation or the examples for a while.

if you run the docs site through any half decent link verifier, you'll find dozens of 404 errors - some are nofollow example.com links which are only for show and can be discounted, but there are multiple links to cpanel pages that are broken.... and the least the left hand should be able to know is what the right hand is doing!

13 hours ago, yggdrasil said:

I would not say it's poor in general, WHMCS has better documentation that other competitors but if they are posting code examples which people use as the base for everything else, that should work. No excuses with that.

yep - if you're going to release a product that requires PHP knowledge to change a menu(!), then the least they can do is provide working coding examples of the basics.

the one I always remember, and I don't think it's there anymore, was a sidebar hook that looked up weather forecasts or temperature by zip code... of course, worked fine for US addresses, but crashed when any outside US address was used.... hardly a good example !

13 hours ago, yggdrasil said:

I actually think I purchased WHMCS because I started reading the documentation on what you could achieve with it. That was years back but this is something they seriously need to improve for developers.

it's a story i've told previously, but the first week or fortnight I got WHMCS back in 2013, I made a list of the errors I kept on finding in the docs - damn thing went of for three sheets of A4 before I gave up...  and i'm not even talking development docs, just installing and configuring the software.

14 hours ago, yggdrasil said:

With open source this is not that important as you can look at the code and debug it. With closed software like this, nobody knows what goes behind the scenes and there is no way to understand how WHMCS does something. For developers documentation is extremely vital. 

one of the many reasons why I gave up on beta testing was the regular occurrence of the relevant dev docs coming out long after the GA had been released... it was a joke situation that got passed being funny.

14 hours ago, yggdrasil said:

I understand you posted many examples here in the community but having to search every post, some that contains bugs as people posted before you post the right one, without mentioning that some are for older versions is annoying.

well, you could filter on only my posts when i've mentioned a particular hook, e.g ClientAreaPrimarySidebar, and that will find only my PS posts.

with regards to hooks for older versions, you might have to remind me with an old example of one that wouldn't work in the latest versions - I don't think the fundamentals of menu hooking have changed that much since 2015.

14 hours ago, yggdrasil said:

I would rather have proper documentation and examples on the WHMCS docs page.

and I wouldn't disagree with that - but we're not in control of that, but I am in control of what I post here and elsewhere.

although even if I wrote complete docs and examples on menu hooking and published them on one of my sites, it would be against the forums rules to link to it as it would be classed as self promotion / advertising... in years gone by, I might have written a tutorial here, but those days are long gone.