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

Make panel scope isolate, and some cleanup #9335

Merged
merged 10 commits into from
Dec 7, 2016

Conversation

stacey-gammon
Copy link
Contributor

  • Make panel scope isolate
  • Refactor and clean up, extracting some logic out of angular code.

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
@w33ble
Copy link
Contributor

w33ble commented Dec 5, 2016

dashboard_panel_directive.js

One thing we want to avoid is unhelpful naming of files. The use of directive in the filename here isn't really useful, since it already lives in a directives directory.

One of the patterns we've discussed using instead is putting everything under a folder with an index.js file that loads any of the associated dependencies and exports what is needed. In this case, you'd end up with:

src/core_plugins/kibana/public/dashboard/directives/dashboard_panel/
  dashboard_panel.js // your existing code
  index.js // exports the code from dashboard_panel.js

When you need to import this file, you simply import the directory path and let the automatic resolution pick up the index:

import { dashboardPanel } from 'kibana/dashboard/directives/dashboard_panel'

We haven't settled on this completely, but the Management team is planning to build our next app this way and see how it goes.

@stacey-gammon
Copy link
Contributor Author

@w33ble, the name change was an attempt to differentiate the directive from the class in components/panel/lib/panel.js which is an object that represents the state of a dashboard panel, separate from any ui specific code (panel.js is an unhelpful difference from dashboard_panel.js, so I didn't want to rely on that and was hoping to eventually give them the same prefix). They are in different folders, so we could keep the name the same, but are we confident we are always going to have a directives folder? The folder structure as it is is a mishmash of folders-by-feature and folders-by-type, so I would like to eventually be consistent, which means they both may end up eventually residing in the same folder.

So my main question is, how do you think we should name business logic vs ui components? dashboard_panel.js and dashboard_panel_ui.js?

@w33ble
Copy link
Contributor

w33ble commented Dec 5, 2016

are we confident we are always going to have a directives folder?

No. It's something we're trying though. So far it's been the best solution we've heard, and if it works out well, we'll push for this change in Kibana core and make it part of the actual styleguide.

The more important part is avoiding naming that doesn't actually provide any value, like I think _directive does here.

The folder structure as it is is a mishmash of folders-by-feature and folders-by-type, so I would like to eventually be consistent, which means they both may end up eventually residing in the same folder.

Yeah, because the rules keep changing. Early on there really were no rules at all, and nothing was ever updated to reflect the decisions we made as we went along. So we have a whole bunch of different patterns and nobody seems motivated to fix any of the old code 😢.

So my main question is, how do you think we should name business logic vs ui components? dashboard_panel.js and dashboard_panel_ui.js?

Based on our plans, you can put whatever you want inside the dashboard_panel/ path, and anything you put there is kind of private. Unless it gets exposed via the index.js file, since that's the canonical definition of what gets exported.

This would be totally acceptable:

src/core_plugins/kibana/public/dashboard/directives/dashboard_panel/
  dashboard_panel.js // your existing code
  index.js // exports the code from dashboard_panel.js
  panel.js

Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

This is a good step towards untangling the panel code. Here are some aspects
that I would see as further improvements in future PRs:

the Panel object: Separating the state from the directive is a great idea.
Only the $el in there feels awkward, as it violates the state-representation
boundary. Maybe it could be tracked in a separate mapping from panel id to dom
element? That would also make the awkward mutation in makeSerializable
unnecessary. An additional idea would be to get rid of the constructor and
factory and just use a createPanelState(...) function that returns a plain
state object.

the panel directive: Avoiding the dependency on the parent controller in
requires: '^dashboardGrid' by using attributes is great. We could go further
and replace the link function with a controller that holds the local state
while employing bindToController and controllerAs to keep the scope as
clean as possible (see
https://github.com/johnpapa/angular-styleguide/blob/master/a1/README.md#style-y076).
A further improvement would be to avoid the parentUiState attributes, because
it retains some of the tight coupling between the panel and the parent
directive. Instead, we could pass appropriate actions to the panel that
encapsulate the necessary manipulation of the parent state in a predictable
and safe way just like with the remove action.

These suggestions probably exceed the scope of this PR, which is already a
great improvement.

remove: '&'
},
link: function ($scope, element) {
if (!$scope.panel.id || !$scope.panel.type) return;
Copy link
Member

Choose a reason for hiding this comment

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

What is supposed to happen when we return here? Could this silence some error conditions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does seem like this could benefit from an exception throw but I'm a little wary to do that without understanding more. That would be a functional change and I was trying to keep it to style changes only, so I just kept the original logic. Maybe there are some corner cases that would result in an empty id or type and we want to keep dashboards working.

* panel.
* @type {PersistedState}
*/
parentUiState: '=',
Copy link
Member

@weltenwort weltenwort Dec 6, 2016

Choose a reason for hiding this comment

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

The parentUiState couples this to the parent quite tightly, because there is no limitation on how it can be manipulated. See the overall review comment for more thoughts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree and more great suggestions above, I'll see about implementing them in another PR!

Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

I agree with @weltenwort that this is a great step towards untangling the panel code.

}

// when gridster tell us it made a change, update each of the panel objects
function readGridsterChangeHandler(e, ui, $widget) {
// ensure that our panel objects keep their size in sync
gridster.$widgets.each(function (i, el) {
const panel = getPanelFor(el);
refreshPanelStats(panel);
panel.$scope.$broadcast('resize');
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we no longer need to $broadcast('resize')?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing it was intentional since it seemed like no one was listening to it. I couldn't find any $scope.$on('resize', though I do see $window.on('resize', safeLayout); but I'm pretty sure that doesn't listen to the broadcast events... I'll have to double check this to make sure though...

Copy link
Contributor

@w33ble w33ble Dec 6, 2016

Choose a reason for hiding this comment

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

FWIW, $broadcast, $emit, $on, and all the $ prefixed lifecycle methods on $scope and don't live outside of scope.

$window should just be a wrapper for the native window, which makes it observe the digest cycle and provides some of the jQuery methods. So I believe you're right, $window.on should have nothing to do with $scope.$broadcast

<li>
<dashboard-panel remove="removePanelFromState(${panel.panelId})"
panel="getPanelByPanelId(${panel.panelId})"
isFullScreenMode="!chrome.getVisible()"
Copy link
Contributor

Choose a reason for hiding this comment

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

isFullScreenMode should be is-full-screen-mode to work https://docs.angularjs.org/guide/directive#normalization

Copy link
Member

Choose a reason for hiding this comment

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

good catch... I make that mistake all the time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dang, nice catch! Wish the linter would catch that, that's the second time I've forgotten about that rule.

@stacey-gammon
Copy link
Contributor Author

@w33ble, in your final example, this:

src/core_plugins/kibana/public/dashboard/directives/dashboard_panel/
  dashboard_panel.js // your existing code
  index.js // exports the code from dashboard_panel.js
  panel.js

doesn't work IMO, because panel.js isn't a directive so it shouldn't go in the directives folder, and I don't think panel.js vs dashboard_panel.js is a good enough distinction. I can see panel.js being exported in the future. Nothing is using it now but one of our goals is to be able to easily rip out one ui framework with another, in which case I think panel.js should be re-useable by any ui code.

We could maybe do the following (ignoring index.js for now, since I'm assuming both will be exported):

src/core_plugins/kibana/public/dashboard/directives/dashboard_panel.js
src/core_plugins/kibana/public/dashboard/lib/dashboard_panel.js

But then we end up with two things of the same name - both called DashboardPanel, and if you are importing them both somewhere it gets confusing (especially since we have named exports and want to use the same name everywhere). Postfixing with an additional description fixes this. Check out for example, how this flux chat app example names their files - they postfix all of them, ChatConstants.js, MessageStore.js, ChatMessageActionCreators.js, etc. Not saying it's right or wrong, but that is one method of doing it which eliminates the above problem.

I'd love to have a longer discussion of our desired folder structure. If we can settle on something, I have the motivation to fix it up! :)

@stacey-gammon
Copy link
Contributor Author

@weltenwort - re: Panel object: Separating the state from the directive is a great idea. Only the $el in there feels awkward, as it violates the state-representation boundary. Maybe it could be tracked in a separate mapping from panel id to dom element? That would also make the awkward mutation in makeSerializable unnecessary. An additional idea would be to get rid of the constructor and factory and just use a createPanelState(...) function that returns a plain state object.

Agree about $el, definitely makes it awkward, and like your suggestion of a separate mapping.

I'm not 100% convinced of your last suggestion. Maybe this isn't a good enough reason, but I like having a named object because I can then use in comments @param {Panel} to specify the type of argument, or return value, and a reader knows where to learn what that object represents and what kind of information it contains.

@weltenwort
Copy link
Member

weltenwort commented Dec 6, 2016

I see - are you using some tool that relies on that specific object attribute/comment syntax? Otherwise nothing would keep you from commenting a createPanelState() function as in

/**
 * Represents a panel on a grid. Keeps track of position in the grid and what visualization it
 * contains.
 * @typedef {Object} PanelState
 * @property {number} id The panel id
 * @property {number} size_x Width of the panel.
 * @property {string} type Type of visualization this panel contains
 */

/**
 * Creates and initializes a basic panel state
 * @param  {number}
 * @param  {string}
 * @return {PanelState}
 */
function createPanelState(id, type) {
  return {
    id,
    size_x: DEFAULT_PANEL_WIDTH,
    type,
  };
}

That should even work with typescript or some other jsdoc tool. The notion of adding complexity by introducing constructors and factories just to document stuff feels weird to me.

- Get rid of factory function and Panel class.
- Rename panel.js to panel_state.js
- Rename dashboard_panel_directive to dashboard_panel
@stacey-gammon
Copy link
Contributor Author

@weltenwort nice! Like PanelState as the name better too. Changes made, thanks for the suggestions.

@w33ble Now that panel.js has a new name, I got rid of the directive postfix. Still would be interested in a more thorough exploration of folder structure one of these days!

@w33ble
Copy link
Contributor

w33ble commented Dec 6, 2016

panel.js isn't a directive so it shouldn't go in the directives folder
I can see panel.js being exported in the future.

I'll admit that I haven't really looked into panel.js, but if it's code that would be used elsewhere, then yes, it should probably be its own thing (service, helper, whatever). If it's not actually meant to be reused, then it's really a kind of helper for the dashboard_panel, in which case, putting it in that directive's folder makes sense. That folder is just a way to encapsulate the directive and any supporting code around it.

src/core_plugins/kibana/public/dashboard/lib/dashboard_panel.js

Sure, if whatever's in there is meant to be reused outside of this one directive. I wish we had something better than lib to call those things though...

But then we end up with two things of the same name - both called DashboardPanel, and if you are importing them both somewhere it gets confusing (especially since we have named exports and want to use the same name everywhere). Postfixing with an additional description fixes this. Check out for example, how this flux chat app example names their files - they postfix all of them.

Yeah, that's an option, but the suffix stuff is something we wanted to try to avoid. I think mostly because the John Papa guide recommended sometimes adding "service" to a filename, but there wasn't any other rules around it. At the very least, I think we should always do it, or never do it, and at least that example is consistent.

img

Still would be interested in a more thorough exploration of folder structure one of these days!

I'll let you know when we settle on something ;). Sorry to muddy up this PR, but thanks for the input.

@stacey-gammon
Copy link
Contributor Author

I think I addressed everything, let me know if not. I'd like to get this in today if possible, so I can do some more work in here without having to deal with a messy merge.

Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

Great improvements! :)

export class PanelUtils {
/**
* Fills in default parameters where not specified.
* @param panel {Panel}
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is of type PanelState instead?


/**
* Ensures that the panel object has the latest size/pos info.
* @param panel {Panel}
Copy link
Member

Choose a reason for hiding this comment

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

type PanelState as well

* $el is a circular structure because it contains a reference to it's parent panel,
* so it needs to be removed before it can be serialized (we also don't
* want it to show up in the url).
* @param panel
Copy link
Member

Choose a reason for hiding this comment

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

PanelState? (three time's the charm)

Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

Nice! LGTM

@stacey-gammon stacey-gammon merged commit 050fc87 into elastic:master Dec 7, 2016
elastic-jasper added a commit that referenced this pull request Dec 7, 2016
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
stacey-gammon pushed a commit that referenced this pull request Dec 14, 2016
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
@trevan
Copy link
Contributor

trevan commented Dec 19, 2016

This appears to have caused #9558.

@stacey-gammon stacey-gammon deleted the panel-refactor-cleanup branch January 9, 2017 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants