Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Closes #10300: Fix Baidu search telemetry #19127

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

rxumoz
Copy link
Contributor

@rxumoz rxumoz commented Apr 20, 2021

Since there might be two types of urls from Baidu (https://m.baidu.com/s?from=... or https://m.baidu.com/from=*/s?), this patch handles baidu separately in Utils.kt

However, there is still a problem that the codes from Baidu are started with numbers (e.g. 844b/1000969a/...), which would be regarded as invalid characters by Glean.

Need more discussion about this, so leave it a draft.

@rxumoz rxumoz linked an issue Apr 20, 2021 that may be closed by this pull request
@gabrielluong gabrielluong added the pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on label Apr 20, 2021
@rxumoz
Copy link
Contributor Author

rxumoz commented May 18, 2021

Since the problem of Baidu that the codes start with figures, I add prefix to these codes with "_"

@rxumoz rxumoz marked this pull request as ready for review May 18, 2021 09:07
@rxumoz rxumoz requested review from a team as code owners May 18, 2021 09:07
@rxumoz rxumoz added needs:review PRs that need to be reviewed and removed pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on labels May 21, 2021
@gabrielluong gabrielluong self-assigned this Jun 2, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2021

Codecov Report

Merging #19127 (91fbb4c) into master (7d5582a) will decrease coverage by 0.01%.
The diff coverage is 64.28%.

❗ Current head 91fbb4c differs from pull request most recent head 95aee9e. Consider uploading reports for the commit 95aee9e to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19127      +/-   ##
============================================
- Coverage     35.79%   35.78%   -0.02%     
  Complexity     1649     1649              
============================================
  Files           545      545              
  Lines         21896    21905       +9     
  Branches       3263     3267       +4     
============================================
  Hits           7838     7838              
- Misses        13151    13161      +10     
+ Partials        907      906       -1     
Impacted Files Coverage Δ
...n/java/org/mozilla/fenix/search/telemetry/Utils.kt 61.90% <50.00%> (-4.77%) ⬇️
...illa/fenix/search/telemetry/BaseSearchTelemetry.kt 87.87% <100.00%> (ø)
...c/main/java/org/mozilla/fenix/home/HomeFragment.kt 2.38% <0.00%> (-1.00%) ⬇️
...in/java/org/mozilla/fenix/perf/NavGraphProvider.kt 62.50% <0.00%> (ø)
...c/main/java/org/mozilla/fenix/ext/AtomicInteger.kt 100.00% <0.00%> (+20.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d5582a...95aee9e. Read the comment docs.

@rxumoz rxumoz requested a review from gabrielluong June 3, 2021 05:12
@@ -22,6 +22,21 @@ internal fun getTrackKey(

if (provider.codeParam.isNotEmpty()) {
code = uri.getQueryParameter(provider.codeParam)
Copy link
Member

Choose a reason for hiding this comment

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

I can imagine us assigning this to a codeParam variable. I wonder how far that could get us without introducing the originalCode, but continuing to modify code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't quite get. Do you mean adding the "_" after the codeParam, or adding other parameters in SearchProviderModel apposed with codeParam?

@gabrielluong gabrielluong added pr:approved PR that has been approved and removed needs:review PRs that need to be reviewed labels Jun 3, 2021
@rxumoz rxumoz added the pr:needs-landing PRs that are ready to land [Will be merged by Mergify] label Jun 7, 2021
@gabrielluong gabrielluong merged commit 3a4f73b into master Jun 8, 2021
@bors bors bot deleted the cn-master-baiduTracking branch June 8, 2021 21:59
grigoryk pushed a commit to grigoryk/fenix that referenced this pull request Jun 15, 2021
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this pull request Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:approved PR that has been approved pr:needs-landing PRs that are ready to land [Will be merged by Mergify]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In_content search telemetry for Baidu not tracked
3 participants