Show the JS tracking code instead of the dashboard while no visit is tracked #7365

mnapoli · 2015-03-05T01:55:00Z

After creating a website (whether it is after the installation or after adding a new website to an existing install), you will see this page as long as no visits are recorded:

mnapoli · 2015-03-05T01:56:02Z

plugins/CoreHome/Controller.php

+        $count = Db::fetchOne($sql, array($this->idSite));
+
+        return $count == 0;
+    }


This function shouldn't be in the controller, could someone please tell me what is the best place for it?

Should this maybe all (Controller etc) be in the SitesManager plugin that listens to the Controller.CoreHome.index event? Not sure if that works. There could be also an event listener in the SitesManager to Request.getRenamedModuleAndAction and rename module and action if needed (if it is CoreHome.index and has no tracking request). In theory CoreHome does not know anything about tracking code etc.

This SQL could go into a separate class eg in Piwik\Tracker\Model but could be also a class in the SitesManager plugin etc.

Maybe also apply a Limit 1 to the query as we only want to know whether there was at least one tracking request.

Thanks! I've done the changes and moved everything to SitesManager and used the Request.dispatch event as it seems to be exactly made for that. Controller.CoreHome.index didn't allow to change the controller.

Maybe also apply a Limit 1 to the query as we only want to know whether there was at least one tracking request.

the limit would apply to the rows returned, given COUNT(*) returns one row it has no effect.

it should be possible to do something like SELECT idvisit FROM piwik_log_visit WHERE idsite=? limit 1 then it should have an effect.

Actually, there is an index on idsite, visit_last_action_time and idsite, idvisitor. So it would be even better to use for example SELECT idvisitor FROM piwik_log_visit WHERE idsite=? limit 1since it then can read from the index which would be even faster.

Edit: Just noticed there is a PRIMARY index on idvisit so doesn't matter...

…it is tracked

mnapoli · 2015-03-06T01:28:13Z

OK to be merged, the UI test failed for a random reason (http://builds-artifacts.piwik.org/ui-tests.no-data-welcome/10722.7/screenshot-diffs/singlediff.html?processed=../processed-ui-screenshots/ActionsDataTable_unflattened.png&expected=ActionsDataTable_unflattened.png&github=ActionsDataTable_unflattened.png)

mattab · 2015-03-06T05:19:06Z

quick note regarding the look and feel: can you make the H2 look like the admin H2:

and H3 look like admin H3:

Btw maybe somehow it's related to inconsistency in https://github.com/PiwikPRO/plugin-CloudAdmin/issues/44 (not sure if that's correct though)

mnapoli · 2015-03-08T21:45:42Z

Done, screenshot updated in the description.

mattab · 2015-03-09T00:20:30Z

Nice improvement... here is feedback

add link on the final sentence %1$sTracking Code%2$s to link to the Tracking code page in admin (which offers more option and may be useful to some)
"No data is recorded yet" -> "No data has been recorded yet"
maybe we can split the translation string SiteWithoutDataHelp into two strings, in case we could reuse one of them later or changing only one of them would be slightly easier for translators.
could we add a UI test for this page, to ensure this onboarding improvement won't regress later on?

mnapoli · 2015-03-09T04:13:27Z

Changes implemented. I've added a test too (3 hours for a simple UI test :'( )

For the record master is broken so the pr is failing

mattab · 2015-03-09T06:34:50Z

plugins/SitesManager/tests/UI/EmptySite_spec.js

+    var generalParams = 'idSite=4&period=day&date=2010-01-03';
+
+    before(function () {
+        testEnvironment.pluginsToLoad = ['SitesManager'];


@diosmosis is it expected we have to manually load the actual plugin the test is written in? could we improve this to automatically load the plugin being tested (here the UI test is in the SitesManager plugin)

no, tests should be run as ./console tests:run-ui --plugin=SitesManager for plugins. howeve, SitesManager is a core plugin, it should always be loaded.

Ah OK I will remove that line then, I just used the UI tests generator and that was added automatically.

mattab · 2015-03-09T06:37:24Z

On the screenshot it seems the H3 looks blue rather than black in the admin?
Btw I'm wondering, would the correct fix be to remove the H2/H3 style that was added and then it should default to the "standard piwik titles" which should look good by default (if they don't look good, can we fix the default h2/h3 so they look good in all future custom pages) ?

mattab · 2015-03-09T06:38:24Z

Changes implemented. I've added a test too (3 hours for a simple UI test :'( )

Nice, the UI test is very useful.

Wow, 3 hours is a long time. Was the painful part to have to manually load the plugin? This seems like it should not be needed, I've asked Benaka in the comment. Otherwise, do you have suggestions to make this better for other devs in the future?

mnapoli · 2015-03-09T21:27:20Z

The painful part was running the tests at all. I've messed up several things on my own machine by setting up piwik so I don't want to try to run the ui test locally. The vagrant setup isn't good because it checks out an entire new copy of Piwik, so I had to resort using the AWS tests runner but had to fix it and improve it first. And then a big problem was understanding how fixtures work and how I can add new fixtures/site. I've given up on that and found the siteId 4 was empty, I hope it will stay that way. The developer documentation on all that is not helpful.

Regarding fixing the titles, I don't really want to refactor the CSS in that PR, I'm already working on that separately. I can fix the blue color.

diosmosis · 2015-03-10T01:35:02Z

FYI if you want to ensure the site is always empty, you can do this.fixture = 'Piwik\Tests\Framework\Fixture' in the UI test. I don't think the ui tests are significantly documented anywhere; I can add something on developer.piwik.org if there's nothing there already.

mattab · 2015-03-10T01:56:11Z

actually UI tests are already documented in the readme: https://github.com/piwik/piwik/blob/master/tests/README.screenshots.md and in the developer site: http://developer.piwik.org/guides/tests-ui

I was asking what was the pain point to see if we could improve those resources...

mnapoli · 2015-03-10T02:03:02Z

@mattab there is no documentation about fixtures though

mattab · 2015-03-10T02:59:01Z

so the missing doc was Fixtures? that's a good point... do you mind create an issue in developer-documentation so we don't forget about this missing doc? it'd be nice to prevent other devs in the future from feeling this frustration 👍

…o load.

mattab · 2015-03-12T01:32:14Z

Looks good to me, very nice change! this should make a big difference to user onboarding and help minimise a lot of frustrations for new users.

Show the JS tracking code instead of the dashboard while no visit is tracked

…o load.

RMastop · 2015-03-21T07:35:41Z

There might be an issue with sites not having visitors every day. I updated a demo site to the latest beta build and instead of showing the dashboard, Piwik showed the tracking script.
After I used the visitor generator, the dashboard shows me the historic data and the just created data.

mnapoli · 2015-03-21T08:26:03Z

@RMastop are you clearing the log_visit table? Because the query is dead simple it should pick up previous visits: SELECT idsite FROM log_visit WHERE idsite = ? limit 1

RMastop · 2015-03-21T09:08:58Z

@mnapoli that's it, I cleared logs and had only archived data on my instance. What are the ods that this happens in real live? Probably none.

mattab · 2015-03-21T20:44:09Z

Good point, actually this could happen for many users, eg. if you setup the "Delete old logs" feature which deletes old logs.

@mnapoli I think we should follow up before 2.12.0 and put some kind of safety net: maybe we disable this screen if the "delete old logs" feature is enabled?

mnapoli · 2015-03-21T22:00:42Z

@mattab Yes indeed, but that is in the case where the entirety of log_visit is deleted for a site, is that a common scenario?

Anyway:

maybe we disable this screen if the "delete old logs" feature is enabled?

That makes sense, will do on monday first thing.

mnapoli · 2015-03-22T21:20:41Z

@mattab I fixed it in 2aed127 and tested locally

mnapoli reviewed Mar 5, 2015
View reviewed changes

mnapoli added this to the Piwik 2.12.0 milestone Mar 5, 2015

mnapoli mentioned this pull request Mar 5, 2015

Show Tracking Code Generator / Notification as long as no visit for a website is tracked #7087

Closed

#7087 Show the JS tracking code instead of the dashboard while no vis…

b1b6009

…it is tracked

mnapoli force-pushed the no-data-welcome branch from 76115b5 to b1b6009 Compare March 5, 2015 03:51

mnapoli added 4 commits March 6, 2015 10:20

#7087 Query optimization

60562b8

#7087 Added test

cf7f60d

Trying to fix test that pass locally but not on travis

eee6bd1

Trying to fix test that pass locally but not on travis

a17363d

Style update

554da97

mattab mentioned this pull request Mar 9, 2015

Improve New User Onboarding #7170

Closed

mnapoli added 4 commits March 9, 2015 14:50

Improved the text help

aac43da

Merge branch 'master' into no-data-welcome

a398c3c

Added tests for the "empty site welcome"

c9149e2

Merge branch 'master' into no-data-welcome

2273967

Rename screenshot to fix the tests

193d9c6

mattab reviewed Mar 9, 2015
View reviewed changes

Simpler UI test

a997288

Black h3

06f3a4f

diosmosis pushed a commit that referenced this pull request Mar 10, 2015

Refs #7365, add log statement to core\Console.php when plugins fail t…

42da3f8

…o load.

mattab pushed a commit that referenced this pull request Mar 12, 2015

Merge pull request #7365 from piwik/no-data-welcome

ceff420

Show the JS tracking code instead of the dashboard while no visit is tracked

mattab merged commit ceff420 into master Mar 12, 2015

mnapoli deleted the no-data-welcome branch March 12, 2015 05:35

diosmosis pushed a commit that referenced this pull request Mar 15, 2015

Refs #7365, add log statement to core\Console.php when plugins fail t…

b8d500a

…o load.

mnapoli added a commit that referenced this pull request Mar 22, 2015

#7365 Do not show the "welcome" screen if log purging is enabled

2aed127

mattab mentioned this pull request Sep 30, 2015

When some data has been tracked, display Congratulations notification #8873

Open

mattab added the c: i18n For issues around internationalisation and localisation. label Oct 13, 2015

mattab added the answered For when a question was asked and we referred to forum or answered it. label Oct 5, 2016

tsteur mentioned this pull request Mar 31, 2017

Screen "No data has been recorded for this website" is not always displayed #11568

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show the JS tracking code instead of the dashboard while no visit is tracked #7365

Show the JS tracking code instead of the dashboard while no visit is tracked #7365

mnapoli commented Mar 5, 2015

mnapoli Mar 5, 2015

tsteur Mar 5, 2015

mnapoli Mar 5, 2015

tsteur Mar 5, 2015

tsteur Mar 5, 2015

mnapoli commented Mar 6, 2015

mattab commented Mar 6, 2015

mnapoli commented Mar 8, 2015

mattab commented Mar 9, 2015

mnapoli commented Mar 9, 2015

mattab Mar 9, 2015

diosmosis Mar 9, 2015

mnapoli Mar 9, 2015

mattab commented Mar 9, 2015

mattab commented Mar 9, 2015

mnapoli commented Mar 9, 2015

diosmosis commented Mar 10, 2015

mattab commented Mar 10, 2015

mnapoli commented Mar 10, 2015

mattab commented Mar 10, 2015

mattab commented Mar 12, 2015

RMastop commented Mar 21, 2015

mnapoli commented Mar 21, 2015

RMastop commented Mar 21, 2015

mattab commented Mar 21, 2015

mnapoli commented Mar 21, 2015

mnapoli commented Mar 22, 2015

Show the JS tracking code instead of the dashboard while no visit is tracked #7365

Show the JS tracking code instead of the dashboard while no visit is tracked #7365

Conversation

mnapoli commented Mar 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnapoli commented Mar 6, 2015

mattab commented Mar 6, 2015

mnapoli commented Mar 8, 2015

mattab commented Mar 9, 2015

mnapoli commented Mar 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattab commented Mar 9, 2015

mattab commented Mar 9, 2015

mnapoli commented Mar 9, 2015

diosmosis commented Mar 10, 2015

mattab commented Mar 10, 2015

mnapoli commented Mar 10, 2015

mattab commented Mar 10, 2015

mattab commented Mar 12, 2015

RMastop commented Mar 21, 2015

mnapoli commented Mar 21, 2015

RMastop commented Mar 21, 2015

mattab commented Mar 21, 2015

mnapoli commented Mar 21, 2015

mnapoli commented Mar 22, 2015