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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added API endpoint to return the php version info #13646

Merged
merged 4 commits into from Nov 2, 2018

Conversation

Projects
None yet
5 participants
@Toflar
Contributor

Toflar commented Oct 26, 2018

Hey everybody,

I'm using the getMatomoVersion endpoint to see if my Matomo setups are all up-to-date. I would like to also know if the PHP version they're all running on are maintained. Unfortunately there's no endpoint yet for that.

I'm very new to the code base of Matomo but actually very experienced in php and testing. Also followed your setup guide for the tests but when trying to run ./console tests:run unit it tried to connect to the db (which is very strange to me for unit tests) so I didn't explore more on it.

Feel free to guide me to the things I need to do to have this tested, I'll happily update the PR if you like this addition 馃槃

@fdellwing

This comment has been minimized.

Contributor

fdellwing commented Oct 26, 2018

Hey @Toflar

please see https://travis-ci.org/matomo-org/matomo/builds/446616865 for your tests results (not all fails are your fault (but some are), just take a look if anything looks like it could be your fault ;))

@fdellwing

This comment has been minimized.

Contributor

fdellwing commented Oct 26, 2018

@Toflar

This comment has been minimized.

Contributor

Toflar commented Oct 26, 2018

Oh nice, UI tests using screenshots, that's cool!
Anyway, I would've liked to add unit tests but the result is dynamic (depending on the running php version) so I couldn't just add the XML, I would've had to add quite some logic to it and so I tried to see how getMatomoVersion() is tested and it's not at all so :-(

@tsteur

This comment has been minimized.

Member

tsteur commented Oct 28, 2018

FYI: I quickly checked... the DB connection should be only needed to bootstrap the test system but the unit tests itself shouldn't connect to the DB.

*/
public function getPhpVersion()
{
Piwik::checkUserHasSomeViewAccess();

This comment has been minimized.

@tsteur

tsteur Oct 28, 2018

Member

This would need to be Piwik::checkUserHasSuperUserAccess();.

This comment has been minimized.

@Toflar

Toflar Oct 29, 2018

Contributor

Can you elaborate why? I mean, why do I need super user access to get the php version but not to get the matomo version?

This comment has been minimized.

@tsteur

tsteur Oct 29, 2018

Member

We're currently only exposing this information to super users. Partially for security reasons. If you needed it for View users, you could develop a simple plugin that defines this API using eg

./console generate:plugin --name="MyPlugin"
./console generate:api

You could even put the plugin on the Marketplace https://developer.matomo.org/guides/distributing-your-plugin

This comment has been minimized.

@Findus23

Findus23 Oct 29, 2018

Member

I agree with @tsteur.
There are many ways a user with view access could find out the Matomo version without the api, but there should be non to find out the php version.

@tsteur

This comment has been minimized.

Member

tsteur commented Oct 31, 2018

Looks good for me. Any thoughts @mattab ?

@mattab

This comment has been minimized.

Member

mattab commented Nov 2, 2018

LGTM 馃憤

@tsteur

This comment has been minimized.

Member

tsteur commented Nov 2, 2018

Cheers @Toflar

@tsteur tsteur merged commit b178978 into matomo-org:3.x-dev Nov 2, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@Toflar Toflar deleted the Toflar:feature/api-php-version branch Nov 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment