Updates to support webtrees 1.7.9

[Ephemerality]

Initial changes required to run the module on webtrees 1.7.9.
Slow day yesterday, so I went ahead and re-did the changes with more granular commits. Hopefully that makes it easier to review :)

[ratatine]

I attempted this PR. FYI, I get an error:
Fatal error: Call to a member function getSignificantIndividual() on null in /usr/local/apache2/htdocs/webtrees/app/Theme/AbstractTheme.php on line 1973
I'm still trying to sort it out so if I find something I will update.

Update; I should have put in this is on 1.7.9

[Ephemerality]

Good to know. I've updated to title to reflect the need to be on 1.7.9.

[ratatine]

Ok correction on my correction... "I should have put in my post that I'm running 1.7.9"... lol "Lets eat, grandma." vs "Lets eat grandma." Grammar and commas save lives.

[Ephemerality]

Oh, I see. I was hoping you figured it out! In the latest version, that line is blank: https://github.com/fisharebest/webtrees/blob/master/app/Theme/AbstractTheme.php#L1973
I wonder where yours differs...

[ratatine]

Very interesting. Mine is different from the master. I wonder if something was skipped during an upgrade? I have:
/**

• Generate a list of items for the main menu.
• @return Menu[]
  */
  protected function primaryMenu() {
  global $controller;

if ($this->tree) {

L1973 ---> $individual = $controller->getSignificantIndividual();

  return array_filter(array_merge(array(
    $this->menuHomePage(),
    $this->menuChart($individual),
    $this->menuLists($controller->getSignificantSurname()),
    $this->menuCalendar(),
    $this->menuReports(),
    $this->menuSearch(),
  ), $this->menuModules()));
} else {
  // No public trees? No genealogy menu!
  return array();
}

}

Let me see about downloading a new set of files and see if that resolves it. Must be something horked up somewhere.

[ratatine]

I just checked and that's not the file distributed with 1.7.9. The diff -u is pretty huge. It still runs if swap that in but the error just moves to line 1983. (The new call.) If you have any suggestions to troubleshoot I'm happy to do so. My first guess is that $controller shouldn't be null but it's hard to track a global you're not familiar with.

[Ephemerality]

$controller definitely shouldn't be null, it should contain the main page controller for the site, but not sure what would be causing it to go away. The only time this uses the global $controller is in the getMenu() function where it only calls a couple functions...
I'm running on the absolute latest commit of the master branch of webtrees, it's entirely possible something has changed since 1.7.9 was released and I'm overlooking.

[ratatine]

I just cloned the latest and my version shows 1.8.0-dev. I'm still getting the same error, FYI. What PHP version are you using?

[Ephemerality]

PHP is version 7.1.5. Just to confirm, if you roll back to the master commit of this repo, does it work? It almost feels as though a global $controller got stuck somewhere it shouldn't be. If you do a search for all the $controller occurrences there should only be 1 global one, in getMenu().

[anmol26s]

Hi
Its exciting to see fixes on this plugin. Is the plugin working and ready to be integrated in Webtrees?
Its very valuable to have an option for connecting through Facebook which could motivate people to join.

[lmbvm]

Hello,
I have the same question like anmol26s. Is it ready for 1.7.9?
It would be very, very useful. Many of mine resources in the family, could be much more engaged with an user friendly login with Facebook.

Thank you.
Luis

[Ephemerality]

@anmol26s @lmbvm It was working for me when I committed it (aside from a couple fixes shorty after). The 1 person who had issues hasn't come back to say if they ever fixed it or not and no one else has said whether it is or isn't working for them, so you're probably best off trying it and seeing how it goes.

[mnoorenberghe]

Thank you very much and apologies about the horribly slow review. I had assumed that a new webtrees version would have been released a long time ago and I was waiting for that to avoid having to update the module yet again. It seems like v2 of webtrees is still a ways away though.

[anmol26s]

Thank you
Will test soon

[mnoorenberghe]

While testing these changes I hit the getSignificantIndividual error that @ratatine hit for new users. I'll try fix it now.

Fatal error: Call to a member function getSignificantIndividual() on null in /Users/…/webtrees-git/app/Theme/AbstractTheme.php on line 1973

[mnoorenberghe]

@Ephemerality Reverting the global $controller; changes from aea9835 (see af05146) fixed the getSignificantIndividual problem. Do you have any recollection of why you changed that? The commit message included "fixed issue w/ global var". Maybe this will break in newer PHP?

[Ephemerality]

Yeah I don't really recall what the issue was with it, I'll have to remember to be more specific with that sort of thing next time. Not sure what the difference is, but mine seems to work with or without the globals defined there.
As a side note - the fetchFriendList function is broken in newer versions of the facebook api, so it's harder to pre-approve users now.

[mnoorenberghe]

Yeah I don't really recall what the issue was with it, I'll have to remember to be more specific with that sort of thing next time. Not sure what the difference is, but mine seems to work with or without the globals defined there.

I think you didn't test the brand new user case on this branch since I fixed a few bugs with that after this PR.

As a side note - the fetchFriendList function is broken in newer versions of the facebook api, so it's harder to pre-approve users now.

Yeah, I'm well aware of that as it's mentioned in the README and covered by #16 and #11. It's really unfortunate and it's part of the reason I lost motivation to work on this module for a while. Facebook and Webtrees are constantly breaking things for no clear reason.

[mnoorenberghe]

Also note that I added Selenium integration tests that interact with the Graph API so that testing different scenarios isn't as tedious and it would mean I can merge future PRs easier. There is still more scenarios to add to the tests.

[Ephemerality]

I think you didn't test the brand new user case on this branch since I fixed a few bugs with that after this PR.

Definitely not. I haven't done much with it at all since June except continuously try to keep it working in the latest dev version (2.0.0). I'll have to check out your updates...

I'm well aware of that as it's mentioned in the README and covered by #16 and #11.

Doh, I missed that, thought it was a more recent change. It's definitely the most annoying breaking change of all.