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

Bug/fix visState title when saving a new visualization #6296

Closed
wants to merge 1 commit into from
Closed

Bug/fix visState title when saving a new visualization #6296

wants to merge 1 commit into from

Conversation

repocho
Copy link
Contributor

@repocho repocho commented Feb 21, 2016

title is not bound to visState object so we need to assign it to be able to save visState properly.

Closes #5901

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@epixa
Copy link
Contributor

epixa commented Feb 25, 2016

jenkins, test it

@epixa epixa added the review label Feb 25, 2016
@repocho
Copy link
Contributor Author

repocho commented May 11, 2016

@LeeDr the original issue was created by you. Perhaps you can review this PR also.
Thanks !

@panda01 panda01 self-assigned this May 11, 2016
@LeeDr
Copy link

LeeDr commented May 11, 2016

LGTM

I tested it and got the title saved;

[
  {
    "_id": "LeeViz-1",
    "_type": "visualization",
    "_source": {
      "title": "LeeViz 1",
      "visState": "{\"title\":\"LeeViz 1\",\"type\":\"area\",\"params\":{\"shareYAxis\":true,\"addTooltip\":true,\"addLegend\":true,\"smoothLines\":false,\"scale\":\"linear\",\"interpolate\":\"linear\",\"mode\":\"stacked\",\"times\":[],\"addTimeMarker\":false,\"defaultYExtents\":false,\"setYExtents\":false,\"yAxis\":{}},\"aggs\":[{\"id\":\"1\",\"type\":\"count\",\"schema\":\"metric\",\"params\":{}},{\"id\":\"2\",\"type\":\"date_histogram\",\"schema\":\"segment\",\"params\":{\"field\":\"@timestamp\",\"interval\":\"auto\",\"customInterval\":\"2h\",\"min_doc_count\":1,\"extended_bounds\":{}}}],\"listeners\":{}}",
      "uiStateJSON": "{}",
      "description": "",
      "version": 1,
      "kibanaSavedObjectMeta": {
        "searchSourceJSON": "{\"index\":\"logstash-*\",\"query\":{\"query_string\":{\"query\":\"*\",\"analyze_wildcard\":true}},\"filter\":[]}"
      }
    }
  }
]

@LeeDr
Copy link

LeeDr commented May 11, 2016

@panda01 if the code looks good to you I think it's good to merge

@panda01
Copy link
Contributor

panda01 commented May 11, 2016

I don't think this is an issue any longer.

@panda01
Copy link
Contributor

panda01 commented May 11, 2016

titlearound

@panda01 panda01 closed this May 11, 2016
@repocho
Copy link
Contributor Author

repocho commented May 11, 2016

@panda01

In your gif the title is not correct inside the visState object. This was the issue actually:
"visState": "{\"title\":\"New Visualization\" ...

I don't know the importance of this visState object when exporting it, but the issue is not solved in your gif.

@panda01
Copy link
Contributor

panda01 commented May 11, 2016

Whoops. Hmm I didn't see that honestly, but I can't see the point in it being in two places. Perhaps we should just remove it from the visState

@panda01
Copy link
Contributor

panda01 commented May 11, 2016

Hmm, so if you resubmit this pr, I'll get ti merged for you. I also was having trouble testing this, because in the merge from field, it says unknown repo. maybe you deleted the branch?

@repocho
Copy link
Contributor Author

repocho commented May 12, 2016

Sorry @panda01 but as you said I've deleted the branch and I don't have the time to do this again.

@epixa
Copy link
Contributor

epixa commented May 12, 2016

We can recreate it from the remote reference.

@epixa
Copy link
Contributor

epixa commented May 12, 2016

Reissued in #7185

In case anyone's curious:

git fetch upstream pull/6296/head
git checkout FETCH_HEAD
git checkout -b fix-visstate-title

@repocho
Copy link
Contributor Author

repocho commented May 12, 2016

In case anyone's curious:

git fetch upstream pull/6296/head
git checkout FETCH_HEAD
git checkout -b fix-visstate-title

Great ! Thanks @epixa

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.

5 participants