Skip to content

Adds "Segmented Visitor Log" icon to more reports and subtables #9478

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

Merged
merged 2 commits into from
Jan 12, 2016
Merged

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jan 5, 2016

fixes #9501

@sgiehl sgiehl added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Jan 5, 2016
@sgiehl sgiehl added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Jan 6, 2016
@sgiehl sgiehl force-pushed the 7213 branch 2 times, most recently from d46189d to fe1d3b5 Compare January 6, 2016 17:52
@@ -1,7 +1,279 @@
<?xml version="1.0" encoding="utf-8" ?>
<results>
<result idSite="1">
<result date="2010-01-03" />
<result date="2010-01-03">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might need to modify https://github.com/piwik/piwik/blob/2.16.0-b1/tests/PHPUnit/System/TwoVisitsWithCustomVariablesSegmentMatchNONETest.php#L58-L77 and maybe add a special case so it doesn't match anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll try to adjust that.

@sgiehl sgiehl force-pushed the 7213 branch 5 times, most recently from 3791977 to 67e540b Compare January 11, 2016 23:33
@tsteur
Copy link
Member

tsteur commented Jan 11, 2016

PR looks good! Can you maybe squash the commits and then merge? If not a direct merge should be ok too

@tsteur tsteur added this to the 2.16.0 milestone Jan 11, 2016
@tsteur tsteur mentioned this pull request Jan 12, 2016
sgiehl added a commit that referenced this pull request Jan 12, 2016
Adds "Segmented Visitor Log" icon to more reports and subtables
@sgiehl sgiehl merged commit 3f8c476 into master Jan 12, 2016
@sgiehl sgiehl deleted the 7213 branch January 12, 2016 18:34
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8" ?>
<result>
<row>Unknown</row>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgiehl would it be possible to have this test generate real values so it shows that the auto suggest for this new segment works as expected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll issue a pull request for that...

@mattab mattab added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Jan 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review
Development

Successfully merging this pull request may close these issues.

3 participants