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

Use english locale explicitly when finding widget subclass. #11637

Merged
merged 2 commits into from
Nov 18, 2021

Conversation

dennisoelkers
Copy link
Member

@dennisoelkers dennisoelkers commented Nov 15, 2021

Description

Motivation and Context

Note: This requires a backport to 4.2.

Prior to this PR we were using the default locales when converting widget types to lower case. For the turkish language, this can lead to certain characters being changed in an unexpected way.

This PR is now using the en locale explicitly when lowercasing the widget type.

Thanks to @supahgreg for his work in #11601.

Fixes #11596.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@dennisoelkers dennisoelkers added this to the 4.2.2 milestone Nov 15, 2021
@@ -108,7 +108,7 @@ class Widget {
streams,
} = this._value;

return { id, type: type.toLocaleLowerCase(), config, filter, timerange, query, streams };
return { id, type: type.toLowerCase(), config, filter, timerange, query, streams };
Copy link
Contributor

@supahgreg supahgreg Nov 17, 2021

Choose a reason for hiding this comment

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

Oh yeah, that's much better than .toLocaleLowerCase('en'). Went ahead and updated #11601, just in case.

Copy link
Contributor

@linuspahl linuspahl left a comment

Choose a reason for hiding this comment

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

Looks good to me, fixes the described problem!

@linuspahl linuspahl merged commit 98c1b86 into master Nov 18, 2021
@linuspahl linuspahl deleted the fix/issue-11596 branch November 18, 2021 10:40
linuspahl pushed a commit that referenced this pull request Nov 18, 2021
* Use english locale explicitly when finding widget subclass.

* Simplifying fix. Thanks @linuspahl!
linuspahl pushed a commit that referenced this pull request Nov 18, 2021
* Use english locale explicitly when finding widget subclass.

* Simplifying fix. Thanks @linuspahl!
dennisoelkers added a commit that referenced this pull request Nov 18, 2021
…11657)

* Use english locale explicitly when finding widget subclass.

* Simplifying fix. Thanks @linuspahl!

Co-authored-by: Dennis Oelkers <dennis@graylog.com>
dennisoelkers added a commit that referenced this pull request Nov 18, 2021
…11658)

* Use english locale explicitly when finding widget subclass.

* Simplifying fix. Thanks @linuspahl!

Co-authored-by: Dennis Oelkers <dennis@graylog.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unknown Widget: aggregation error on different computers
3 participants