Skip to content
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

Jump to bottom

[Marketing]adding help link or search tracking campaign params #18601

Merged
merged 34 commits into from Mar 7, 2022
Merged

[Marketing]adding help link or search tracking campaign params #18601

merged 34 commits into from Mar 7, 2022

Conversation

peterhashair
Copy link
Contributor

@peterhashair peterhashair commented Jan 11, 2022 •
edited

Description:

adding help link or search tracking campaign params.
Fixes: #18598

Review

Peter and others added 5 commits January 11, 2022 17:56
update vue and twig and php functions
d8fd384 
update vue and twig and php functions
Update helpLink.ts
d4c5b78 
update regular expression
@peterhashair
built vue files
ebf909f
update xml
b126215 
update xml
Merge branch 'm-18598-add-tracking-url' of github.com:matomo-org/mato…
21afae9 
…mo into m-18598-add-tracking-url
sgiehl
sgiehl requested changes Jan 11, 2022
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

Left a couple of comments for improvements.
Btw. did you think about solving this issue in javascript only? Instead of having the links with all campaign parameters within the API responses, we could have a new angular/vue component, which is used instead of a normal <a> tag. And when it's rendered it can automatically parse the link and append the campaign parameters if needed...

core/Category/Subcategory.php Outdated Show resolved Hide resolved
core/Piwik.php Outdated Show resolved Hide resolved
plugins/CoreHome/vue/src/helpLink.ts Outdated Show resolved Hide resolved
plugins/Feedback/templates/index.twig Outdated Show resolved Hide resolved
Peter added 3 commits January 12, 2022 10:33
update some place
e5b0fbe 
update some place
Merge branch '4.x-dev' into m-18598-add-tracking-url
d31265f 
# Conflicts:
#	plugins/CoreHome/vue/src/SiteSelector/SiteRef.ts
add tests and group all into one javascript
0f8259a 
add tests and group all into one javascript
@peterhashair
Copy link
Contributor Author

peterhashair commented Jan 11, 2022 •
edited

did you think about solving this issue in javascript only?

Yes, group most of them into a vue/angular directory. but the getHelp() doesn't like span matomo-help-link base="https://matomo.org/docs/" text="test"></span> or VUE version <helpLink base="url" text='test'/>, not too sure why, but update the regex, should be better

Peter and others added 8 commits January 12, 2022 12:44
Update HelpLink.vue
586c4ee 
update vue build
@peterhashair
built vue files
17526ab
update some style and xml fixes
6513827 
update some style and xml fixes
@peterhashair
built vue files
af95f6f
Update FrontControllerTest.php
da4cea6 
update exception error
Merge branch '4.x-dev' into m-18598-add-tracking-url
1df65cc 
# Conflicts:
#	tests/PHPUnit/Integration/FrontControllerTest.php
Update FrontControllerTest.php
c408486 
update tests
Update FrontControllerTest.php
e086a11 
remove all the line numbers
@peterhashair peterhashair added the Needs Review PRs that need a code review label Jan 13, 2022
@peterhashair peterhashair added this to the 4.7.0 milestone Jan 13, 2022
@peterhashair peterhashair added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jan 13, 2022
sgiehl
sgiehl previously requested changes Jan 13, 2022
View reviewed changes
plugins/API/WidgetMetadata.php Outdated Show resolved Hide resolved
plugins/API/WidgetMetadata.php Outdated Show resolved Hide resolved
js/piwik.js Outdated Show resolved Hide resolved
core/Twig.php Outdated Show resolved Hide resolved
plugins/CoreHome/vue/src/HelpLink/HelpLink.vue Outdated Show resolved Hide resolved
@sgiehl sgiehl removed the Needs Review PRs that need a code review label Jan 13, 2022
Peter and others added 3 commits January 14, 2022 12:45
Merge branch '4.x-dev' into m-18598-add-tracking-url
a049acb 
# Conflicts:
#	plugins/CoreHome/vue/dist/CoreHome.umd.js
#	plugins/CoreHome/vue/dist/CoreHome.umd.min.js
update to hardcode campaign params
f8d861b 
update to hardcode campaign params
@peterhashair
built vue files
c858efc
Peter added 3 commits January 18, 2022 13:34
Peter and others added 4 commits February 1, 2022 10:30
update tests
4934828 
update tests
@peterhashair
built vue files
6a5e2e6
Revert "update tests"
7ad5bd3 
This reverts commit 4934828.
update xml
53cfc7c 
update xml
@justinvelluppillai justinvelluppillai modified the milestones: 4.7.0, 4.8.0 Feb 2, 2022
Merge branch '4.x-dev' into m-18598-add-tracking-url
8708b6f 
# Conflicts:
#	tests/PHPUnit/Integration/FrontControllerTest.php
@peterhashair peterhashair added the Needs Review PRs that need a code review label Feb 2, 2022
@peterhashair peterhashair changed the title adding help link or search tracking campagin params [Marketing]adding help link or search tracking campaign params Feb 9, 2022
Merge branch '4.x-dev' into m-18598-add-tracking-url
ad2ed89 
# Conflicts:
#	plugins/CoreHome/vue/dist/CoreHome.umd.min.js
@github-actions
Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Feb 23, 2022
@peterhashair peterhashair removed the Stale The label used by the Close Stale Issues action label Feb 23, 2022
peterhashair and others added 3 commits February 24, 2022 00:26
@peterhashair
built vue files
2960543
Merge branch '4.x-dev' into m-18598-add-tracking-url
c109063 
# Conflicts:
#	plugins/CoreHome/vue/dist/CoreHome.umd.min.js
@peterhashair
built vue files
cbbc48a
@justinvelluppillai justinvelluppillai modified the milestones: 4.8.0, 4.9.0 Mar 1, 2022
@peterhashair peterhashair requested review from justinvelluppillai and removed request for justinvelluppillai March 1, 2022 23:21
justinvelluppillai
justinvelluppillai approved these changes Mar 7, 2022
View reviewed changes
Copy link
Contributor

@justinvelluppillai justinvelluppillai left a comment

Choose a reason for hiding this comment

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

Looking good here, I've just made one suggestion to avoid 301s when I click two of the links changed.

Peter Zhang and others added 2 commits March 8, 2022 10:24
@justinvelluppillai
Update plugins/Feedback/templates/index.twig
3025f36 
Co-authored-by: Justin Velluppillai <justin@innocraft.com>
@justinvelluppillai
Update plugins/Feedback/templates/index.twig
4785bf4 
Co-authored-by: Justin Velluppillai <justin@innocraft.com>
@peterhashair peterhashair merged commit 284c380 into 4.x-dev Mar 7, 2022
@peterhashair peterhashair deleted the m-18598-add-tracking-url branch March 7, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diosmosis diosmosis diosmosis left review comments

@sgiehl sgiehl sgiehl left review comments

@justinvelluppillai justinvelluppillai justinvelluppillai approved these changes

@tsteur tsteur Awaiting requested review from tsteur

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.9.0
Development

Successfully merging this pull request may close these issues.

Add URL campaign parameters to in-app search and help links
5 participants
@peterhashair @diosmosis @tsteur @justinvelluppillai @sgiehl