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

If user settings cannot be found for installed version of Kibana, turn Kibana plugin red #7333

Merged
merged 3 commits into from
Jun 2, 2016
Merged

If user settings cannot be found for installed version of Kibana, turn Kibana plugin red #7333

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4902271
If there is an error, return an empty object for user settings
ycombinator May 31, 2016
357e391
Turn Kibana plugin red if user settings cannot be found
ycombinator Jun 1, 2016
484e45f
Using _.partial() instead of bind()
ycombinator Jun 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/ui/settings/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ export default function setupSettings(kbnServer, server, config) {
return client
.get({ ...clientSettings })
.then(res => res._source)
.then(user => hydrateUserSettings(user));
.then(user => hydrateUserSettings(user))
.catch(e => { return {}; });
Copy link
Contributor

Choose a reason for hiding this comment

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

You can do .catch(e => ({})); instead. While .catch(e => {}) would result in an empty block function, ({}) is treated as an expression resolving to an empty object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, how interesting! I tend to prefer the more explicit syntax so its obvious to the reader what's going on but this is good to know!

Copy link
Contributor

Choose a reason for hiding this comment

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

Returning an object from an arrow function is a fairly common scenario, so I'd expect most people to be aware of what's going on (or be able to infer it from context at least).

By the way I would move the .catch call one level up, so that it doesn't muck with hydrateUserSettings, which has no expected reason for throwing even if an ES error is caught.

Copy link
Contributor Author

@ycombinator ycombinator Jun 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Good call on moving the .catch call one level up. Since we are also going to be logging here, I'm just going to create a small function to do that and return the {}, and pass that function to .catch. Hopefully this will help make it clearer why this bit of code exists.

}

function setMany(changes) {
Expand Down