Save/rename flow fix #9087

stacey-gammon · 2016-11-15T20:51:20Z

Use auto generated ids instead of coupling the id to the title which
creates problems.
Adjust UI to make the save flow more understandable.
Remove confirmation on overwrite since we will now be creating
duplicate objects even if they have the same title.

cjcenizal

This is fantastic! I have some UX concerns and minor code suggestions. Do you think we can add some tests for saved_object.js?

cjcenizal · 2016-11-16T00:32:31Z

src/ui/public/courier/saved_object/saved_object.js

+     * Returns true if the object's original title has been changed. New objects return false.
+     * @return {boolean}
+     */
+    self.titleChanged = function () {


Minor nitpick: can we call this isTitleChanged? I really like the convention of prefixing flags and boolean-returning functions with is, has, will, can, etc. to reflect their role.

nice suggestion, done!

cjcenizal · 2016-11-16T00:44:29Z

src/core_plugins/kibana/public/dashboard/partials/save_dashboard.html

@@ -18,5 +18,7 @@
      <kbn-info info="Change the time filter to the currently selected time each time this dashboard is loaded"></kbn-info>
    </label>
  </div>
-  <button type="submit" ng-disabled="!opts.dashboard.title" class="btn btn-primary" aria-label="Save dashboard">Save</button>
+  <button type="submit" ng-disabled="!opts.dashboard.title" class="btn btn-primary" aria-label="Save dashboard">
+    {{opts.dashboard.titleChanged() ? 'Save a Copy' : 'Save'}}


I tested the UX of this, and I think that by making the title editable immediately, we are making it feel like the user can rename the dashboard. When the user then starts changing the name, the button changes to "Save a Copy". I feel like there's a high possibility that this will confuse users: "Why did that button just change? What did I do to cause it to change? How do I change it back? Hm, maybe it was like that all along??"

In this situation, I think we really need to be careful to put the user in full control of the app. This means the UI should change in reaction to very explicit commands from the user. In this case, the user's explicit command was to change the name of the dashboard -- not to disable "Save" and enable "Save As". Even though it makes sense to us and the application internally, the rationale is opaque to the user.

I think we should use the Courté Checkbox here. When the user clicks the checkbox, s/he is very explicitly commanding the app to save a new document, so a change in the UI in response to this command makes sense.

I pushed a change to the dashboard save panel so you can play around with both options. Ignore the extremely ugly code and style for now - I want to settle on which variation to use before taking time to clean it up.

I understand your reservations about the button, definitely makes sense that a user may be confused about why the button text changed and how to get it back. I still like it better than the text box - IMO just the lesser of two evils and it's equally unclear that checking the checkbox will enable the text box. The flow seems extremely random, especially for a brand new user (well, both would seem like pretty random side affects to a new user). We could add help text when the button changes to make it clearer... maybe a little warning icon and 'Saving a change to the title of your dashboard will create a copy of it. Use the original name if you wish to save the current object without creating a copy. '

Gah, both are pretty darn ugly. At least whichever we choose will hopefully be a very short lived solution!

I definitely defer to you and the design team for whichever you prefer, so just let me know and I will clean it up and address the rest of the great comments here! :)

cjcenizal · 2016-11-16T00:45:47Z

src/ui/public/courier/saved_object/saved_object.js

@@ -55,11 +55,10 @@ export default function SavedObjectFactory(es, kbnIndex, Promise, Private, Notif
    let customInit = config.init || _.noop;

    // optional search source which this object configures
-    self.searchSource = config.searchSource && new SearchSource();
+    self.searchSource = config.searchSource ? new SearchSource() : null;


Do we need to use null here? If not, then I think undefined is more accurate.

I disagree. The way I see it, self.searchSource is undefined until the correct value is chosen, and then it's defined and explicitly set to a "nothing" value.

I'd like to avoid a null vs undefined debate, and I can see we're using null in a million places in the codebase, so I don't really care about its use here. Unfortunately we're also using loose equality to compare values against it, which renders it a bit moot. 😄

But I would like to know if null has a role in any logic in this context? E.g. if (someVal === nulll) doSomething(). I agree with you if a "nothing" value actually matters... but if not, then ¯\_(ツ)_/¯ Seems odd but I don't really care.

I disagree with parts of both sides :)

I do like Stacy's style... I much rather have a one liner than introduce another if branch in the code (if/else branches account to noise)

I don't like the use of null and I much rather only using undefined. Yes, I know that some argue against it and say you shouldn't do it, but I'm +100 to also explicitly assigning it when appropriate. Why? because it's explicit. JS as a lang as this core characteristic that variable values are hidden assignments are hard to follow. The more explicit we have things, the easier it is to read & understand the code.

but I'm +100 to also explicitly assigning it when appropriate

Me too. When do you think it's appropriate?

I personally feel like undefined is only appropriate for things that are uninitialized or where the correct value can not be determined. undefined === "not sure" null === "nothing"

for me both null and undefined are "nothing".

Sure we can make this more "sophisticated" and say that "nothing" is "something" and therefore there are different types of "nothing" - a "nothing" that is really a something (a value) and a nothing that is really a nothing and therefore we're not sure what that nothing is because... well.. it's nothing. Then we can have a discussion around whether an undefined argument should really be a nothing or a something, and whether all the optional args should have a default value of "real" nothing - null... otherwise they'll be really nothing, therefore unknown... and what do you do with unknown? is there a special treatment for unknown vs. known but nothing? (/me taking a break to consult Descartes's essays...)

alternatively, we can just say that nothing is nothing, and both null and undefined are nothing and we can just choose one of them for the sake of simplicity. And since undefined is more ingrained in the language (e.g. optional arguments and uninitialised vars will be undefined by default) we'll just use undefined and ban the use of null... just another existential thought...

Switched to undefined which has the added benefit of maintaining logic of the old code.

cjcenizal · 2016-11-16T00:47:00Z

src/ui/public/courier/saved_object/saved_object.js

+     * The goal is to move towards a better rename flow, but since our users have been conditioned
+     * to expect a 'save as' flow during a rename, we are keeping the logic the same until a better
+     * UI/UX can be worked out.
+     */


Awesome comments!

cjcenizal · 2016-11-16T00:48:59Z

src/ui/public/courier/saved_object/saved_object.js

     */
+    self.save = function () {
+      self.forceSaveAsOnTitleChange();
+      self.ensureId();


forceSaveAsOnTitleChange conditionally calls ensureId. Why do we call it twice? Do we even need forceSaveAsOnTitleChange? Maybe we can just put that logic here.

Yea, like your suggestion better, done.

uboness · 2016-11-16T16:40:37Z

Didn't look too in-depth at the change, but a few comments:

Make sure we don't break BWC (existing dashboards are still loaded and saved with their current id)
We do have a problem with the top menu bar, said, it is what it is today... I would not do too much to change it outside of the work of changing the whole header in kibana. If it really doesn't fit, we can consider having a checkbox right above the "save" button - "[ ] Store as a new dashboard" (just above the other checkbox that we have there to store the time"
make sure this plays well with editing a dashboard in "Management/Kibana/Saved Objects/Dashboard" editing functionality.

cjcenizal · 2016-11-16T17:05:16Z

@stacey-gammon What do you think of this Codepen? http://codepen.io/cjcenizal/pen/ENgeag

This is what I had in mind by changing the appearance of the text input. It basically changes from regular text to an editable input, so the change in affordance is more apparent. And unchecking the box after editing the text input reverts it back to its original state.

stacey-gammon · 2016-11-16T17:17:35Z

I do like that better @cjcenizal, but keep in mind there is a header above the current text input boxes which might make it look weird. Would you want me to remove that?

stacey-gammon · 2016-11-16T17:21:48Z

One more thing to keep in mind - we also already have the title displayed at the top, so it might look repetitive if we get rid of "Save as" and just have a label header when the checkbox is unchecked.

cjcenizal · 2016-11-16T17:29:48Z

@stacey-gammon I think it makes sense to just have a "Save" label at the top. I updated the Codepen. What do you think?

spalger · 2016-11-16T18:39:37Z

@cjcenizal I think the codepen makes the checkbox unmissable, which is good, but how do you think the "Store time with dashboard" checkbox should fit?

cjcenizal · 2016-11-16T18:41:58Z

@spalger I know we're trying to make as few UI changes as possible, but you have a really good point -- it doesn't really make sense here. The context is weird. I think we should move it into Options, which IMO makes more sense for this kind of, well, option.

spalger · 2016-11-16T18:48:18Z

@epixa I seem to remember that you were pretty strongly against moving the "Store time with dashboard" option

epixa · 2016-11-16T19:13:52Z

I'm not sure where we should move the option, and I don't want to move it somewhere now only to move it again later.

uboness · 2016-11-17T01:39:32Z

It feels like we're complicating things... I would not move the "time checkbox" anywhere now. We need to narrow the scope of the change as much as possible.

Whats's wrong with the following?

We shouldn't try to fix the whole UI/UX in this PR... but focus on the functional flow of how Save/Save As should work. Sure, we need to eventually fix the UI/UX as well, but that should be a separate dedicated PRs.

spalger · 2016-11-17T01:51:05Z

If the checkbox mimics the behavior we have been shipping for years then I'm good with that @uboness.

When the save panel is opened the checkbox should be unchecked
If the user changes the title the checkbox should become checked

uboness · 2016-11-17T02:04:40Z

For years ppl got used to a crappy experience, where they want to rename a dashboard and they all of a sudden get two copies of the dashboard. At some point they noticed that... and then they realized that "hey... this is the only way I can rename a dashboard... save it, go to "settings/saved objects/dashboard", figure out what is the old one a what is the new one, and delete the old one"

is this the default functionality/experience you want to preserve ^?

spalger · 2016-11-17T02:11:28Z

Maybe I'm crazy and nobody uses the save panel to create a new object based on another object.

That's the functionality that I think people will expect if they open the save panel and it looks almost exactly like it used to.

spalger · 2016-11-17T02:16:38Z

Imagine this use case:

User A wants to modify a panel they see on a shared dashboard
clicks the edit button
Makes some changes
decides they want to add it to their personal dashboard
Saves the visualization with a new name
Adds it to the dashboard
goes to bed

The next day

User B comes and looks at the shared dashboard and for some reason one of the visualizations had been changed by User A and the old visualization is lost.

This is what I imagine happening if we change the default behavior for the save button without making some other obvious change.

spalger · 2016-11-17T02:20:19Z

Also, if we want to make as few changes as possible then we don't need to change anything. When we index the saved visualization and the title has changed we don't send an id, and the "save as a copy" behavior will be preserved and renaming objects with the object manager will actually work

uboness · 2016-11-17T02:31:52Z

@spalger the scenario you describes is a great example of why we should change the current behaviour... We cannot "fix" multi-tenancy by redefining the notion of saving an object. Every application I've ever used had a very clear distinction between "Save" & "Save As". In every app/system I ever used, if there's a "default" save action, or a primary save action (a more accessible save action), it was/is always just "Save".. and "Save As" is a secondary action. And Kibana decided to change that... reeducated users with years of training & education around one of the most basic functionalities ever exposed on a computer screen.... we changed it.

Multi tendency needs to be solved with object level security and proper Kibana level ACLs & capabilities privileges, not by redefining how the world should work.

Also, if we want to make as few changes as possible then we don't need to change anything

yes.. indeed, that is always an option in everything that we do

spalger · 2016-11-17T09:44:33Z

Talked to @uboness about this out of band and we think that maybe we should show a "deprecation warning" when you change the name of a visualization and don't have the "Save as new..." checkbox checked.

mockup: http://codepen.io/spalger/pen/dOOYxW

I think of this warning like a deprecation warning that we'll remove once we rework the whole save UX, or after some amount of time. We might also want to include an advanced setting to disable it if people think it's annoying.

uboness · 2016-11-17T13:58:10Z

We'll need to fine tune the message and the styling, but I'm good with @spencer's latest proposal

stacey-gammon · 2016-11-17T14:38:41Z

ditto, I'll work on making the adjustments soon as I am done writing up
some tests for saved_object.js

On Thu, Nov 17, 2016 at 8:58 AM, uboness [email protected] wrote:

We'll need to fine tune the message and the styling, but I'm good with
@spencer https://github.com/spencer's latest proposal

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#9087 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/APy9k8K2tHR_Jp5iP0384lOcUk_1TCoqks5q_F17gaJpZM4KzAZp
.

stacey-gammon · 2016-11-17T19:01:55Z

@cjcenizal Tests added in a separate PR - #9127. This way I can be sure my changes here haven't unintentionally broken any existing behavior.

tbragin · 2016-11-20T21:21:25Z

This PR so far introduces different workflows for dashboards vs visualizations and saved searches. I assume we plan to unify them, once we settle on the approach?

Do we want to make this change in Timelion save dialog as well?

Regarding the notice @spalger proposed, assuming we modify saved workflows for saved searches, visualizations, and dashboards in one go, I fail to see how it's necessary. Even though there is a change in workflow, the resulting UI will be clear on "Save" vs "Save As" actions, whereas previously they were conflated. @spalger I don't understand the scenario you describe in this comment. If in step 5 the user saves the visualization with a new name, they are creating a new object, so how would the existing dashboard pointing to the existing visualization be changed as a result? Perhaps I'm missing something.

I also worry a bit about the precedent of introducing deprecation notices in the UI for every workflow modification... seems like that would get messy quickly.

uboness · 2016-11-21T09:37:28Z

s/"Copy on Save"/"Save as new Dashboard"

Also, the save button needs to be under the checkboxes

@tbragin it's not a deprecation message.. It's a change of behavior message. I think it's a valid point that we take an existing functionality that's been around for quite some time and change its default behavior... We should warn the user. Note that the only reason we need to do that is due to the incoherent UI we have now. Once we fix the UI, action names & buttons should speak for themselves without the need for special warning messages.

We should fine tune and shorten the message text

stacey-gammon · 2016-11-21T13:48:47Z

Yep @tbragin & @uboness, absolutely going to unify the workflows and clean up the UI to @spalger's recommendation above. I wanted to give people a chance to play around with both variations to help determine which one works best and the UI with the checkbox on the same level as the button was thrown in as a very quick first iteration to give reviewers a chance to compare the two work flows. Was busy writing unit tests last week (and took Friday off) but will get started on it today!

@tbragin - I don't think we should change the UI for the Timelion save as Kibana dashboard panel option (only the first option). You can't actually edit the dashboard panel so if you save in timelion it will always be a save as, and that input text box will never be pre-filled. For example, create a new timelion sheet and click save as dashboard panel. Then click open - you will not see the sheet you just saved as an option to select. It will only be an option when you try to open a visualization. If you add that panel to a dashboard and click edit, it takes you to visualize, not timelion.

stacey-gammon · 2016-11-21T16:30:48Z

@cjcenizal, Should Dashboard always be capitalized? In the proposed ui, we have Save as new Dashboard right above Store time with dashboard. I think we should be consistent in the capitalization.

cjcenizal · 2016-11-30T17:38:15Z

Looks awesome!

spalger

The warning message needs to use savedObject.getDisplayName() in another place, and it looks like the issue from #9087 (review) is still a problem

spalger · 2016-11-30T20:47:42Z

src/ui/public/courier/saved_object/ui/saved_object_save_as_checkbox.html

@@ -0,0 +1,9 @@
+<div ng-hide="!savedObject.id || savedObject.isSaving">
+  <div ng-hide="!savedObject.isTitleChanged() || savedObject.copyOnSave" class="localDropdownWarning">
+    In previous versions of Kibana, changing the name of a Dashboard would make a copy with the new name. Use the 'Save as a new {{savedObject.getDisplayName()}}' checkbox to do this now.


This shouldn't have Dashboard hard coded

don't hardcode Dashboard

stacey-gammon · 2016-11-30T21:38:01Z

Gaaahh, sorry for letting that slip, and thank you for the thorough review! Updated!

spalger

LGTM

…_object_refactor

Backports PR #9087 **Commit 1:** Save/rename flow fix - Use auto generated ids instead of coupling the id to the title which creates problems. - Adjust UI to make the save flow more understandable. - Remove confirmation on overwrite since we will now be creating duplicate objects even if they have the same title. * Original sha: faf3e9c * Authored by Stacey Gammon <[email protected]> on 2016-11-15T20:45:16Z **Commit 2:** use undefined instead of null * Original sha: 08b0e92 * Authored by Stacey Gammon <[email protected]> on 2016-11-21T17:35:56Z **Commit 3:** Change titleChanged function name * Original sha: b7b6e0a * Authored by Stacey Gammon <[email protected]> on 2016-11-21T17:40:35Z **Commit 4:** Merge branch 'master' of https://github.com/elastic/kibana into saved_object_refactor * Original sha: 07901bd * Authored by Stacey Gammon <[email protected]> on 2016-11-21T20:58:09Z **Commit 5:** address code review comments * Original sha: 41441bf * Authored by Stacey Gammon <[email protected]> on 2016-11-22T20:10:09Z **Commit 6:** Merge branch 'master' of https://github.com/elastic/kibana into saved_object_refactor * Original sha: e3ff0ad * Authored by Stacey Gammon <[email protected]> on 2016-11-28T14:43:23Z **Commit 7:** Add isSaving flag to avoid checkbox flicker, fix regression bug from refactor. Added tests and fixed a couple bugs Updated info message * Original sha: cf04dde * Authored by Stacey Gammon <[email protected]> on 2016-11-28T14:45:52Z **Commit 8:** Update doc and nav title with new name on rename don't hardcode Dashboard * Original sha: 9686b2a * Authored by Stacey Gammon <[email protected]> on 2016-11-30T21:08:59Z **Commit 9:** Merge branch 'master' of https://github.com/elastic/kibana into saved_object_refactor * Original sha: cb6f918 * Authored by Stacey Gammon <[email protected]> on 2016-12-02T14:23:30Z

Also clean up and simplify scope destroying Fix sorting on scripted date and boolean fields (elastic#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: elastic#9257 Add docs on all available console server config options (elastic#9288) settings: do not query ES for settings in non-green status (elastic#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (elastic#9313) more refactoring Fix sorting on scripted date and boolean fields (elastic#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: elastic#9257 Add docs on all available console server config options (elastic#9288) settings: do not query ES for settings in non-green status (elastic#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (elastic#9313) Skip assertion when the test blips. (elastic#9251) Tests fails intermittently, and for identical reasons. When this occurs, skip the test. This only occurs in CI and cannot be reproduced locally. Additional changes: - Use async/await to improve legibility. - Move async behaviour to beforeEach and keep test stubs synchronous to improve labeling of tests. [git] ignore extra files in the root config/ directory (elastic#9296) upgrade makelogs (elastic#9295) build: remove deepModules hackery (elastic#9327) The deepModules hacks in the build system were added to support the long paths that resulted from npm2, but npm3 fundamentally addresses that problem, so deepModules is no longer necessary. In practical terms, npm3 shouldn't ever cause path lengths to become so long that they trigger path length problems on certain operating systems. more cleanup and tests Save/rename flow fix (elastic#9087) * Save/rename flow fix - Use auto generated ids instead of coupling the id to the title which creates problems. - Adjust UI to make the save flow more understandable. - Remove confirmation on overwrite since we will now be creating duplicate objects even if they have the same title. * use undefined instead of null * Change titleChanged function name * address code review comments * Add isSaving flag to avoid checkbox flicker, fix regression bug from refactor. Added tests and fixed a couple bugs Updated info message * Update doc and nav title with new name on rename don't hardcode Dashboard

Backports PR #9087 **Commit 1:** Save/rename flow fix - Use auto generated ids instead of coupling the id to the title which creates problems. - Adjust UI to make the save flow more understandable. - Remove confirmation on overwrite since we will now be creating duplicate objects even if they have the same title. * Original sha: faf3e9c * Authored by Stacey Gammon <[email protected]> on 2016-11-15T20:45:16Z **Commit 2:** use undefined instead of null * Original sha: 08b0e92 * Authored by Stacey Gammon <[email protected]> on 2016-11-21T17:35:56Z **Commit 3:** Change titleChanged function name * Original sha: b7b6e0a * Authored by Stacey Gammon <[email protected]> on 2016-11-21T17:40:35Z **Commit 4:** Merge branch 'master' of https://github.com/elastic/kibana into saved_object_refactor * Original sha: 07901bd * Authored by Stacey Gammon <[email protected]> on 2016-11-21T20:58:09Z **Commit 5:** address code review comments * Original sha: 41441bf * Authored by Stacey Gammon <[email protected]> on 2016-11-22T20:10:09Z **Commit 6:** Merge branch 'master' of https://github.com/elastic/kibana into saved_object_refactor * Original sha: e3ff0ad * Authored by Stacey Gammon <[email protected]> on 2016-11-28T14:43:23Z **Commit 7:** Add isSaving flag to avoid checkbox flicker, fix regression bug from refactor. Added tests and fixed a couple bugs Updated info message * Original sha: cf04dde * Authored by Stacey Gammon <[email protected]> on 2016-11-28T14:45:52Z **Commit 8:** Update doc and nav title with new name on rename don't hardcode Dashboard * Original sha: 9686b2a * Authored by Stacey Gammon <[email protected]> on 2016-11-30T21:08:59Z **Commit 9:** Merge branch 'master' of https://github.com/elastic/kibana into saved_object_refactor * Original sha: cb6f918 * Authored by Stacey Gammon <[email protected]> on 2016-12-02T14:23:30Z

* make scope isolate * Make panel scope isolate Also clean up and simplify scope destroying Fix sorting on scripted date and boolean fields (#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: #9257 Add docs on all available console server config options (#9288) settings: do not query ES for settings in non-green status (#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (#9313) more refactoring Fix sorting on scripted date and boolean fields (#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: #9257 Add docs on all available console server config options (#9288) settings: do not query ES for settings in non-green status (#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (#9313) Skip assertion when the test blips. (#9251) Tests fails intermittently, and for identical reasons. When this occurs, skip the test. This only occurs in CI and cannot be reproduced locally. Additional changes: - Use async/await to improve legibility. - Move async behaviour to beforeEach and keep test stubs synchronous to improve labeling of tests. [git] ignore extra files in the root config/ directory (#9296) upgrade makelogs (#9295) build: remove deepModules hackery (#9327) The deepModules hacks in the build system were added to support the long paths that resulted from npm2, but npm3 fundamentally addresses that problem, so deepModules is no longer necessary. In practical terms, npm3 shouldn't ever cause path lengths to become so long that they trigger path length problems on certain operating systems. more cleanup and tests Save/rename flow fix (#9087) * Save/rename flow fix - Use auto generated ids instead of coupling the id to the title which creates problems. - Adjust UI to make the save flow more understandable. - Remove confirmation on overwrite since we will now be creating duplicate objects even if they have the same title. * use undefined instead of null * Change titleChanged function name * address code review comments * Add isSaving flag to avoid checkbox flicker, fix regression bug from refactor. Added tests and fixed a couple bugs Updated info message * Update doc and nav title with new name on rename don't hardcode Dashboard * namespace panel factory cleanup * Fix parameter name in html * Address comments - Get rid of factory function and Panel class. - Rename panel.js to panel_state.js - Rename dashboard_panel_directive to dashboard_panel * Fix file path reference in tests. * Panel => PanelState

Backports PR #9335 **Commit 1:** make scope isolate * Original sha: c54eb3f * Authored by Stacey Gammon <[email protected]> on 2016-11-30T20:55:28Z **Commit 2:** Make panel scope isolate Also clean up and simplify scope destroying Fix sorting on scripted date and boolean fields (#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: #9257 Add docs on all available console server config options (#9288) settings: do not query ES for settings in non-green status (#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (#9313) more refactoring Fix sorting on scripted date and boolean fields (#9261) The elasticsearch API only [supports][1][2] sort scripts of type `number` and `string`. Since dates need to be returned as millis since the epoch for visualizations to work anyway, we can simply add a condition to send dates as type number in the sort API. ES will cast booleans if we tell them its a string, so we can add a similar condition there as well. [1]: https://www.elastic.co/guide/en/elasticsearch/reference/5.0/search-request-sort.html#_script_based_sorting [2]: https://github.com/elastic/elasticsearch/blob/aeb97ff41298e26b107a733837dfe17f123c0c9b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java#L359 Fixes: #9257 Add docs on all available console server config options (#9288) settings: do not query ES for settings in non-green status (#9308) If the ui settings status is not green, that means there is at least one dependency (so elasticsearch at the moment) that is not met in order for it to function correctly, so we shouldn't attempt to determine user settings at all. This ensures that when something like the version check fails in the elasticsearch plugin, Kibana correctly behaves by not attempting requests to elasticsearch, which prevents 500 errors and allows users to see the error status on the status page. We also now periodically check for compatible elasticsearch versions so that Kibana can automatically recover if the elasticsearch node is upgraded to the appropriate version. Change loading screen background to white to make it less distracting when navigating between apps. (#9313) Skip assertion when the test blips. (#9251) Tests fails intermittently, and for identical reasons. When this occurs, skip the test. This only occurs in CI and cannot be reproduced locally. Additional changes: - Use async/await to improve legibility. - Move async behaviour to beforeEach and keep test stubs synchronous to improve labeling of tests. [git] ignore extra files in the root config/ directory (#9296) upgrade makelogs (#9295) build: remove deepModules hackery (#9327) The deepModules hacks in the build system were added to support the long paths that resulted from npm2, but npm3 fundamentally addresses that problem, so deepModules is no longer necessary. In practical terms, npm3 shouldn't ever cause path lengths to become so long that they trigger path length problems on certain operating systems. more cleanup and tests Save/rename flow fix (#9087) * Save/rename flow fix - Use auto generated ids instead of coupling the id to the title which creates problems. - Adjust UI to make the save flow more understandable. - Remove confirmation on overwrite since we will now be creating duplicate objects even if they have the same title. * use undefined instead of null * Change titleChanged function name * address code review comments * Add isSaving flag to avoid checkbox flicker, fix regression bug from refactor. Added tests and fixed a couple bugs Updated info message * Update doc and nav title with new name on rename don't hardcode Dashboard * Original sha: e76f638 * Authored by Stacey Gammon <[email protected]> on 2016-12-01T18:54:50Z **Commit 3:** Merge branch 'master' of https://github.com/elastic/kibana into panel-refactor-cleanup * Original sha: 82323be * Authored by Stacey Gammon <[email protected]> on 2016-12-02T15:23:56Z **Commit 4:** namespace panel factory cleanup * Original sha: 3a09d66 * Authored by Stacey Gammon <[email protected]> on 2016-12-02T15:31:09Z **Commit 5:** Fix parameter name in html * Original sha: 93afafa * Authored by Stacey Gammon <[email protected]> on 2016-12-06T14:25:51Z **Commit 6:** Address comments - Get rid of factory function and Panel class. - Rename panel.js to panel_state.js - Rename dashboard_panel_directive to dashboard_panel * Original sha: aa8d6c8 * Authored by Stacey Gammon <[email protected]> on 2016-12-06T15:27:58Z **Commit 7:** Merge branch 'master' of https://github.com/elastic/kibana into panel-refactor-cleanup * Original sha: cfd610a * Authored by Stacey Gammon <[email protected]> on 2016-12-06T16:04:56Z **Commit 8:** Fix file path reference in tests. * Original sha: 317e76e * Authored by Stacey Gammon <[email protected]> on 2016-12-06T18:11:42Z **Commit 9:** Merge branch 'master' of https://github.com/elastic/kibana into panel-refactor-cleanup * Original sha: a6288dd * Authored by Stacey Gammon <[email protected]> on 2016-12-07T14:22:36Z **Commit 10:** Panel => PanelState * Original sha: c87f45b * Authored by Stacey Gammon <[email protected]> on 2016-12-07T14:32:08Z

cjcenizal self-assigned this Nov 15, 2016

spalger assigned cjcenizal and spalger and unassigned cjcenizal Nov 15, 2016

cjcenizal suggested changes Nov 16, 2016

View reviewed changes

tbragin added the :Sharing label Nov 16, 2016

cjcenizal approved these changes Nov 30, 2016

View reviewed changes

stacey-gammon added the v5.2.0 label Nov 30, 2016

spalger suggested changes Nov 30, 2016

View reviewed changes

stacey-gammon force-pushed the saved_object_refactor branch from b54b31b to d7aa70e Compare November 30, 2016 21:33

Update doc and nav title with new name on rename

9686b2a

don't hardcode Dashboard

stacey-gammon force-pushed the saved_object_refactor branch from d7aa70e to 9686b2a Compare November 30, 2016 21:36

spalger approved these changes Dec 2, 2016

View reviewed changes

Merge branch 'master' of https://github.com/elastic/kibana into saved…

cb6f918

…_object_refactor

stacey-gammon merged commit f432698 into elastic:master Dec 2, 2016

stacey-gammon deleted the saved_object_refactor branch December 2, 2016 14:34

elastic-jasper mentioned this pull request Dec 2, 2016

[5.x] Save/rename flow fix #9333

Merged

cjcenizal mentioned this pull request Dec 5, 2016

Add date created, date modified, other metadata to Saved Objects #9202

Open

6 tasks

tbragin mentioned this pull request Dec 6, 2016

Inconsistent saved object management experience #9056

Closed

elastic-jasper mentioned this pull request Dec 7, 2016

[5.x] Make panel scope isolate, and some cleanup #9407

Merged

tbragin mentioned this pull request Jan 4, 2017

Dashboard-specific visualization title #3393

Closed

This was referenced Jan 6, 2017

Ability to rename visualizations in the Viz app #7720

Closed

Streamline rename of saved objects #2735

Closed

marius-dr mentioned this pull request Jan 24, 2017

Tilemaps always make the appStatus dirty #8768

Closed

JalehD mentioned this pull request Feb 13, 2017

possibility to create multiple dashboards with the same name doesn't give a user friendly experience. #10307

Closed

epixa mentioned this pull request Mar 18, 2017

[FeatureRequest] add a "rename" option when saving existing visualization with different name #10759

Closed

cjcenizal mentioned this pull request Apr 19, 2017

Human readable dashboard ids #11338

Closed

tylersmalley mentioned this pull request Nov 14, 2017

Feature request: "Save copies of edited dashboards/visualizations" #14935

Closed

Save/rename flow fix #9087

Save/rename flow fix #9087

Conversation

stacey-gammon commented Nov 15, 2016

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal Nov 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalger Nov 16, 2016 • edited Loading

Choose a reason for hiding this comment

cjcenizal Nov 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalger Nov 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uboness commented Nov 16, 2016

cjcenizal commented Nov 16, 2016 • edited Loading

stacey-gammon commented Nov 16, 2016

stacey-gammon commented Nov 16, 2016

cjcenizal commented Nov 16, 2016

spalger commented Nov 16, 2016

cjcenizal commented Nov 16, 2016

spalger commented Nov 16, 2016

epixa commented Nov 16, 2016

uboness commented Nov 17, 2016

spalger commented Nov 17, 2016 • edited Loading

uboness commented Nov 17, 2016 • edited Loading

spalger commented Nov 17, 2016

spalger commented Nov 17, 2016 • edited Loading

spalger commented Nov 17, 2016 • edited Loading

uboness commented Nov 17, 2016 • edited Loading

spalger commented Nov 17, 2016 • edited Loading

uboness commented Nov 17, 2016

stacey-gammon commented Nov 17, 2016

stacey-gammon commented Nov 17, 2016

tbragin commented Nov 20, 2016 • edited Loading

uboness commented Nov 21, 2016 • edited Loading

stacey-gammon commented Nov 21, 2016 • edited Loading

stacey-gammon commented Nov 21, 2016

cjcenizal commented Nov 30, 2016

spalger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stacey-gammon commented Nov 30, 2016

spalger left a comment

Choose a reason for hiding this comment

cjcenizal Nov 16, 2016 •

edited

Loading

spalger Nov 16, 2016 •

edited

Loading

cjcenizal Nov 16, 2016 •

edited

Loading

spalger Nov 16, 2016 •

edited

Loading

cjcenizal commented Nov 16, 2016 •

edited

Loading

spalger commented Nov 17, 2016 •

edited

Loading

uboness commented Nov 17, 2016 •

edited

Loading

spalger commented Nov 17, 2016 •

edited

Loading

spalger commented Nov 17, 2016 •

edited

Loading

uboness commented Nov 17, 2016 •

edited

Loading

spalger commented Nov 17, 2016 •

edited

Loading

tbragin commented Nov 20, 2016 •

edited

Loading

uboness commented Nov 21, 2016 •

edited

Loading

stacey-gammon commented Nov 21, 2016 •

edited

Loading