Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Please extend LocationProvider #8230

Closed
sebastianpiskorski opened this issue Jun 26, 2015 · 2 comments
Closed

Please extend LocationProvider #8230

sebastianpiskorski opened this issue Jun 26, 2015 · 2 comments
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone
2.14.1

Comments

@sebastianpiskorski
Copy link
Contributor

Please extend Location provider https://github.com/piwik/piwik/blob/2.13.1/plugins/UserCountry/LocationProvider.php by method getInfo() which returns Info object that contains getTitle() method.

It was used in EnterpriseAdmin in such manner: https://github.com/PiwikPRO/plugin-EnterpriseAdmin/blob/2.1.0/Commands/Status.php#L237

So without assurance that returned object will have getInfo method, command was dying with error.
@mattab mattab closed this as completed in 80a222c Jul 15, 2015
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jul 15, 2015
@mattab mattab added this to the 2.14.1 milestone Jul 15, 2015
@mattab
Copy link
Member

mattab commented Jul 15, 2015

Hi @sebastianpiskorski

I didn't quite do as you suggested, but simply changed getProvider() signature to return null instead of false, when the provider is invalid. This way we can simply test for !is_null or !empty to check the object is valid before use. it should be good enough for our needs, thanks for suggestion!

diosmosis pushed a commit that referenced this issue Jul 16, 2015
Refs #8230, make sure users of Location::getProviderById do not check…
cd5b74f 
… if the result is false, but is empty.
@sebastianpiskorski
Copy link
Contributor Author

@mattab I think that if we are using an instance of LocationProvider object we should only use methods that are defined in its interface. So we would avoid fatal errors. So if we want to use getInfo() method we should extend LocationProvider interface first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
2.14.1
Development

No branches or pull requests
2 participants
@mattab @sebastianpiskorski