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

Properly handle timestamp field in stacked quickvalues #4516

Merged
merged 5 commits into from Jan 24, 2018

Conversation

Projects
None yet
3 participants
@kroepke
Member

kroepke commented Jan 24, 2018

In certain scenarios the timestamp field was not properly formatted when being used in stacked quickvalues.

If the timestamp field wasn't the first field of the stack, it failed to be formatted.
Additionally, adding it to the query either dropped later fields or misconverted the timezone.

Widget rendering was also affected, although there it cannot be added to a search query so the "Add to query" issue did not happen.

fixes #4509

kroepke and others added some commits Jan 24, 2018

pass all stacked fields into QuickValuesVisualization
with that information we can properly format the timestamp even if it isn't the first field

still has issues with timezone conversions
Pass timestamp to format directly
Stacked fields have a different object structure as regular fields, so
we can't rely on an object key being there.
Do not convert timestamp to UTC
`DateTime` will convert the value to the user timezone, which is what we
want to display.
include stacked fields in quick values title
use correct default prop name
create stacked fields array on the fly if the caller doesn't give us …
…the fields

this is the case for widgets, because the container doesn't know what to pass

@kroepke kroepke added this to the 2.4.2 milestone Jan 24, 2018

@kroepke kroepke self-assigned this Jan 24, 2018

@kroepke kroepke requested review from bernd and edmundoa Jan 24, 2018

@edmundoa

Just saw a tiny cosmetic issue, will open an issue for that and we are good to go on my side.

@bernd

bernd approved these changes Jan 24, 2018

Works for me.

@bernd bernd merged commit 272604a into master Jan 24, 2018

3 of 5 checks passed

ci-web-linter Jenkins build graylog-pr-linter-check 2213 has failed
Details
graylog-project/pr Jenkins build graylog-project-pr-snapshot 955 has failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@wafflebot wafflebot bot removed the ready-for-review label Jan 24, 2018

@bernd bernd deleted the issue-4509 branch Jan 24, 2018

bernd added a commit that referenced this pull request Jan 24, 2018

Properly handle timestamp field in stacked quickvalues (#4516)
* pass all stacked fields into QuickValuesVisualization

with that information we can properly format the timestamp even if it isn't the first field

still has issues with timezone conversions

* Pass timestamp to format directly

Stacked fields have a different object structure as regular fields, so
we can't rely on an object key being there.

* Do not convert timestamp to UTC

`DateTime` will convert the value to the user timezone, which is what we
want to display.

* include stacked fields in quick values title

use correct default prop name

* create stacked fields array on the fly if the caller doesn't give us the fields

this is the case for widgets, because the container doesn't know what to pass

Fixes #4509

(cherry picked from commit 272604a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment