@tsteur opened this Pull Request on September 24th 2018 Member

image

image

@mattab is this what you had in mind?

need to see how to write tests for it.

fix #7724

@tsteur commented on September 24th 2018 Member

FYI: Currently no site values will be saved except for trust cookies... need to figure out a way to make this happen

@mattab commented on September 24th 2018 Member

Feedback:

  • Would change the inline help to This should be enabled on an intranet website for accurate unique visitor tracking. Most visitors viewing an intranet will all have the same IP address, and often the same browser and operating system, which can cause Matomo to under-report the number of visits and unique visitors (if this setting is disabled).
  • could we append eg. "Trust visitors cookies (Recommended)"
  • initially was even thinking to always enable this setting for Intranet website and not even expose it? but more flexible probably to expose it...
@mattab commented on September 24th 2018 Member

Also

  • let's document how to convert a website to an intranet website? Can this be done with a simple SQL query, which one? this will be useful to have somewhere documented (even if just here)
@tsteur commented on September 24th 2018 Member

Much better could be to create a new "Intranet" website type in Core Matomo. For Intranet websites, we directly enable the trust_visitor_cookies setting. This setting becomes a "website setting" de-activable in the UI and visible only for intranet sites.

I'll remove the setting again. must have misunderstood this one.

@tsteur commented on September 24th 2018 Member

Removed the setting again and all related tests. Makes it easier :)

Added tests for the tracking part, should work.

@mattab commented on September 24th 2018 Member

must have misunderstood this one.

Forgot to reread the description so had missed this. But better we take the simplest approach 👍

@tsteur commented on September 25th 2018 Member

Made the changes, will likely need to fix the tests after this

@diosmosis commented on September 28th 2018 Member

Looks like there are some integration test failures. The JsProxyTest one passes for me locally...

@tsteur commented on September 28th 2018 Member

I'll restart the cancelled PR builds

@tsteur commented on September 28th 2018 Member

Locally the test works for me as well

@tsteur commented on October 1st 2018 Member

Test is still failing on travis, locally works. not sure why.

@tsteur commented on October 1st 2018 Member

@diosmosis @mattab @sgiehl do you know why this test would check for url http://localhost//js/?idsite=1 when it says testPiwikPhp in https://github.com/matomo-org/matomo/blob/3.x-dev/tests/PHPUnit/Integration/JsProxyTest.php#L56 ?

fails probably because of a missing htaccess or so? Not sure why it suddenly fails... maybe changing to js/index.php be fine?

@diosmosis commented on October 1st 2018 Member

@tsteur did you find out what the whole error was (ie, why it was getting a 400)?

@tsteur commented on October 1st 2018 Member

Nope not yet. I'll enable debugging and see what happens

@tsteur commented on October 2nd 2018 Member

@diosmosis test should be fixed now

@diosmosis commented on October 2nd 2018 Member

@tsteur there are some QueuedTracking tests still failing, doesn't look expected. Maybe something breaks tracking for queued tracking?

@tsteur commented on October 3rd 2018 Member

@diosmosis should be fixed now

This Pull Request was closed on October 7th 2018
Powered by GitHub Issue Mirror