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
Readable segment values for browser, os and country segments #13929
Conversation
@@ -17,12 +18,23 @@ class BrowserName extends Base | |||
{ | |||
protected $columnName = 'config_browser_name'; | |||
protected $columnType = 'VARCHAR(10) NULL'; | |||
protected $segmentName = 'browserCode'; | |||
protected $segmentName = 'browserName'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this PR is still WIP but be good to not break API and keep the existing segment browserCode
etc. We could also make the segment accept browser names on top besides browserCode or add another segment browserName.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. There will be two segments each browserName
and browserCode
and same for os.
Expected test files may need an update before merging |
I've updated the branch and added an additional segment for the english country name |
@diosmosis browser and os names should now be compared case insensitive |
I'm seeing this change in the tests:
Is that correct? Reporting APIs may need to be changed possibly... |
@diosmosis no, I've updated the code, so that doesn't change. Tests should also be updated now. |
Current segments for browser, os or country are only usable using the short code (e.g. FF, IE, WIN, IOS, de, fr, ...).
refs #5309
fixes #6772
refs #14053