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

Fix unknown keyword is not shown in transitions report in seach engines section #16841

Merged
merged 3 commits into from Dec 11, 2020

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Dec 1, 2020

Description:

Transitions report currently doesn't show the Unknown keyword and therefore the percentages for search keywords are completely wrong. Basically if there are 700 searches, and 5 times the keywords were detected from the search, then it calculates the percentage based on the 5 detected keywords. If 5 keywords were used once, then each keyword gets shown 20% and to the user it looks like 20% of 700 searches.

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@tsteur tsteur added the Needs Review PRs that need a code review label Dec 1, 2020
@tsteur tsteur added this to the 4.1.0 milestone Dec 1, 2020
@diosmosis
Copy link
Member

I guess this isn't tested by an existing system test, should we add one?

@tsteur
Copy link
Member Author

tsteur commented Dec 2, 2020

@diosmosis no tests failed so it's likely not covered. We could add a test but am quite busy and don't really have time to add one and figured at least it'll be fixed. Better than not having it fixed. I reckon ideally there was a test but it's also not crazy important.

@diosmosis
Copy link
Member

@tsteur this is in the 4.1.0 milestone so I think someone could add it later on if there was time

@diosmosis diosmosis merged commit d851713 into 4.x-dev Dec 11, 2020
@diosmosis diosmosis deleted the transitionsunknownsearchengines branch December 11, 2020 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants