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.

Sign up for GitHub

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

Jump to bottom

Display changed field formats without requiring hard page refresh. #52874

Merged
merged 1 commit into from
Dec 13, 2019
Merged

Display changed field formats without requiring hard page refresh. #52874

merged 1 commit into from
Dec 13, 2019

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Dec 12, 2019 •
edited by lukeelmers
Loading

Addresses part of #35235

Summary

Resolves an issue where changing a field format and navigating back to discover/visualizations/dashboard requires a hard page refresh in order to see the changes that were made. This relates to the issue reported in #35235

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@alexwizp alexwizp requested a review from a team as a code owner December 12, 2019 13:10
alexwizp
alexwizp commented Dec 12, 2019
View reviewed changes
src/legacy/ui/public/field_editor/field_editor.js
@@ -727,7 +726,7 @@ export class FieldEditor extends PureComponent {
if (!fieldFormatId) {
indexPattern.fieldFormatMap[field.name] = undefined;
} else {
indexPattern.fieldFormatMap[field.name] = fieldFormat;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

regression of #48108.

indexPattern.fieldFormatMap[field.name] = field.format; - was right

alexwizp
alexwizp commented Dec 12, 2019
View reviewed changes
src/plugins/data/public/index_patterns/fields/field.ts
@@ -94,7 +94,8 @@ export class Field implements IFieldType {
if (!type) type = getKbnFieldType('unknown');

let format = spec.format;
if (!format || !(format instanceof FieldFormat)) {
Copy link
Contributor Author

@alexwizp alexwizp Dec 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Problem was in this line. We inherit all formatter from the FieldFormat but for some reasons format instanceof FieldFormat === false =(

image

@alexwizp alexwizp self-assigned this Dec 12, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@lukeelmers lukeelmers added v7.5.1 bug Fixes for quality problems that affect the customer experience Team:AppArch labels Dec 12, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

lukeelmers
lukeelmers reviewed Dec 12, 2019
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

@alexwizp I'm testing locally and having a hard time getting this to work. Steps to reproduce:

  1. Load web logs sample data
  2. Update index pattern to set ip field to use Url format with a relative hyperlink
  3. Link appears in Discover as expected
  4. But if you create a data table vis, and/or add the table to a dashboard, it isn't working

Am I missing a step?

It would also be great if we could add a functional test for this case, since it is easy to forget to test this manually. I was planning on looking into this today, but I can't really do much without seeing the fix in place.

@alexwizp
Copy link
Contributor Author

@lukeelmers Hm... I've tested only on Discovery. Without my changes we have issue on step 3

@lukeelmers lukeelmers changed the title Kibana 7.0.0 URL field formatter doesn't render relative hyperlinks properly Display changed field formatters without requiring hard page refresh. Dec 12, 2019
@lukeelmers lukeelmers changed the title Display changed field formatters without requiring hard page refresh. Display changed field formats without requiring hard page refresh. Dec 12, 2019
@lukeelmers lukeelmers removed the review label Dec 12, 2019
lukeelmers
lukeelmers approved these changes Dec 12, 2019
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

This LGTM. While it doesn't address the exact use case in #35235, it still addresses a related bug which should be resolved & backported.

Will keep the original issue open until the problem with URL formatters is also resolved.

@alexwizp alexwizp merged commit dc1a26b into elastic:master Dec 13, 2019
alexwizp added a commit to alexwizp/kibana that referenced this pull request Dec 13, 2019
@alexwizp
Kibana 7.0.0 URL field formatter doesn't render relative hyperlinks p…
51ec443 
…roperly (elastic#52874)

Closes elastic#35235
@alexwizp alexwizp mentioned this pull request Dec 13, 2019
Merged
alexwizp added a commit that referenced this pull request Dec 13, 2019
@alexwizp
Kibana 7.0.0 URL field formatter doesn't render relative hyperlinks p…
087db5b 
…roperly (#52874) (#52975)

Closes #35235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukeelmers lukeelmers lukeelmers approved these changes

Assignees

@alexwizp alexwizp

Labels
bug Fixes for quality problems that affect the customer experience release_note:fix v7.5.1 v7.6.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexwizp @elasticmachine @lukeelmers