poitch / dart-matomo

A Dart Matomo Client
MIT License
20 stars 39 forks source link

Add matomo.php postfix to the site URL #14

Open nivisi opened 3 years ago

nivisi commented 3 years ago

Hey 👋 Thanks for the package.

In my project, I spent around an hour trying to figure out why my events doesn't dispatch to matomo. Then I went to the source code and saw that the URL does not contain the matomo.php postfix. I think it should be added in case the customer did not provide it during initialization.

poitch commented 3 years ago

Hi,

I'm not sure to understand where you would want the matomo.php path to be added? The initialize method takes a full URL and the documentation shows a piwik.php.

Could you show me how you initialized MatomoTracker, I guess I could add a check on the provided URLs and log a warning if no path is provided.

nivisi commented 3 years ago

@poitch hey,

The required base URL is: https://my.site.url/matomo.php, however I just used https://my.site.url and nothing worked. Probably I misread the docs, but it would be nice if you'd protect users like me.

Plus, for me it makes more sense if I need to provide the very base url, e.g. https://my.site.url without that matomo.php postfix.

poitch commented 3 years ago

@nivisi makes sense.

I'll have to think about it. Someone can install Matomo in different paths (e.g. https://my.site.url/analytics/matomo.php). But more importantly, two endpoints are possible matomo.php and piwik.php.

Surfacing an error when this library cannot connect in debug mode would probably help discover that more easily.

nivisi commented 3 years ago

@poitch regarding the /analytics thing: on the Matomo they will see the base url along with the site id, won't they? I think it still will work with these changes.

Regarding the two endpoints thing: we could use an enum for this.

poitch commented 3 years ago

If I try and sum up your proposal.

Those would be the potential ways to initialize the tracker

For a matomo instance at the top level:

await MatomoTracker().initialize(
    siteId: siteId,
    url: 'https://my.site.url',
);

For a matomo instance under a path

await MatomoTracker().initialize(
    siteId: siteId,
    url: 'https://my.site.url/some/path',
);

And to handle the matomo.php vs piwik.php case we then would also need something like

await MatomoTracker().initialize(
    siteId: siteId,
    url: 'https://my.site.url/some/path',
    endpoint: MatomoEndpoint.PIWIK,
);

The site ID is passed as an argument parameter, so baseURL?siteId=<siteId>

It seems to me that if a user has to specify https://my.site.url/some/path then why isn't the real endpoint specified with it, I believe it would bring even more confusion to the user of the library.

nivisi commented 3 years ago

@poitch because on the website it says

Base url: https://my.site.url/. // Without the matomo.php.

It's up to you anyway 🙂 I just reported that it was an expected behaviour for me as I had no idea I need to add that matomo.php

JulianBissekkou commented 3 years ago

I would just assert that the final url ends with .php 👍🏽

assert(url.endsWith(".php"));