[Spaces] Space Avatar with selector in main Kibana menu #18609

legrego · 2018-04-26T16:27:07Z

This adds a Global Nav Link to the main Kibana Menu, which will eventually show the currently selected space. It's currently a placeholder space until we can add the correct hooks to detect the active space.

Clicking the link will display a modal allowing the user to change their space.

Space Selection Screen

When security is enabled, this will appear after logging in. Without security, this will be the initial screen users see when navigating directly to Kibana, and not to an app within Kibana:

Main Navigation

The bottom-left navigation region now shows the user's active Space. Clicking this will show a modal allowing them to switch spaces:

Space Selection Modal

Space Management

Quick Visual Walkthrough

Sorry for the low-quality GIF 🥔

TODO:

Design Team UX 🎨

…aces

kobelb · 2018-04-27T15:06:44Z

x-pack/plugins/spaces/index.js

@@ -52,6 +54,42 @@ export const spaces = (kibana) => new kibana.Plugin({
    const config = server.config();
    validateConfig(config, message => server.log(['spaces', 'warning'], message));

+    if (!config.get('xpack.spaces.enabled')) {


I think we shouldn't hit the init if the spaces plugin is disabled, so we should be able to remove this.

kobelb · 2018-05-07T17:04:18Z

x-pack/plugins/spaces/public/lib/spaces_manager.js

+export class SpacesManager {
+  constructor(httpAgent, chrome) {
+    this._httpAgent = httpAgent;
+    this.chrome = chrome;


nit: it doesn't look like this is used after storing it at this.chrome, can we not store a reference to the chrome or rename it to this._chrome?

good catch, I'll fix this

kobelb · 2018-05-07T17:08:34Z

x-pack/plugins/spaces/public/lib/spaces_manager.js

+
+  async updateSpace(space) {
+    return await this._httpAgent
+      .post(`${this._baseUrl}/${space.id}?overwrite=true`, space)


Any reason why are aren't using PUT for updates?

that's an oversight - thanks!

kobelb · 2018-05-07T17:35:39Z

x-pack/plugins/spaces/public/lib/spaces_manager.js

+
+  async getActiveSpace() {
+    if (!this._activeSpace) {
+      this._activeSpace = await this._httpAgent


This is an interesting approach, are we executing a full-page request after switching a space so that we clear this out?

I was thinking that we'd have to do a full page request after switching spaces. If we don't, then there are other implications, like changing the chrome service's basePath, and identifying any places where that might have been cached, such as the left nav bar.

that makes sense, sounds good!

kobelb · 2018-05-07T20:00:30Z

x-pack/plugins/spaces/server/routes/api/v1/spaces.js

+          return reply();
+        }
+
+        return reply(convertSavedObjectToSpace(spaces[0]));


Can we modify this to throw that Error that we talked about if we find more than one space that matches the url-context?

Yes We Can! 👍

What should our behavior be when we detect this? Should a banner display at the top of the screen, should we redirect to the default space, or something else?

I don't think we should allow the user to stay in this Space, because we don't really know which space they intended to visit. If security wasn't an issue, then we could send them to the management screen, but not everybody will have access to it. We could also send them to a dedicated error page that explains what happened...maybe with a link to the management screen?

Similarly, what should the experience be when navigating to a space that doesn't exist?

Most 500s from APIs will cause that toast notification that an error occurred, so doing the same thing here (if possible) makes sense to me. We do have a fatal error page that we could potentially use, but the toast notification is probably more consistent.

kobelb · 2018-05-07T20:03:28Z

Also, the redirect that is occurring from the switching is just going to s/${spaceId} when there isn't a basepath, as opposed to a relative URL.

legrego · 2018-05-08T19:31:11Z

Also, the redirect that is occurring from the switching is just going to s/${spaceId} when there isn't a basepath, as opposed to a relative URL.

Great catch - I fixed this as well. Thanks!

rhoboat · 2018-05-10T20:39:40Z

The flow seems to work well. I'm curious what the design team thinks of a modal vs. another kind of visual motif. I'm curious whether the Delete Space has all the warnings one needs.

Which brings up: what happens when a space is deleted? Objects that were assigned to that space become global again?

(Edit: I think this is a better place for this discussion than #18862 because that is more about SOC than Spaces itself)

legrego · 2018-05-10T20:49:37Z

I'm curious what the design team thinks of a modal vs. another kind of visual motif.

Yeah I don't love the modal. The initial mockups had the space selector in a popover, but I ran into display & positioning issues in the left nav bar, so I swapped it out...at least temporarily. I wanted to get the rest of the PR functional and not churn on that in the short term.

rhoboat · 2018-05-11T12:33:57Z

What about my main question?

kobelb · 2018-05-11T12:39:25Z

The ability to delete a space and all related objects is the one of the main reasons why we scheduled the meeting this afternoon, so hopefully we'll have a better answer after this.

legrego · 2018-05-15T20:22:33Z

Here is the EuiPopover rendering "issue" that I mentioned above with the left nav bar:

The Popover is constrained to the navbar area, and it doesn't seem possible to get it to "break free" and use the rest of the available space.
Maybe someone from @elastic/eui-design can weigh in here?

legrego · 2018-05-18T12:28:45Z

After talking with @kobelb, we decided in the interest of time that we can move forward with the Modal approach. If we would prefer the popover in the future, then we can perhaps reevaluate once we are on the new platform (where the left nav is Reactified)

legrego · 2018-05-18T18:07:52Z

@kobelb This is ready for your review. I haven't scheduled a design review yet, but I want to merge this to our feature branch so that I can move other work forward. I'll do the UI touchups as a separate PR.

I unfortunately scattered the tests across a couple of different PRs, so it's hard to see right now what has tests and what doesn't. In any event, I plan on adding more tests once I have these PRs merged together.

kobelb

Looks great, one concern with our callWithInternalUser usage, but besides that it's looking good.

kobelb · 2018-05-21T14:50:51Z

x-pack/plugins/spaces/server/routes/api/v1/spaces.js

+    };
+  }
+
+  const callWithInternalUser = getClient(server).callWithInternalUser;


I'm assuming it's not possible to use the SavedObjectsClient here? We shouldn't be using callWithInternalUser at least, as this has the potential to grant unauthorized users permission to the spaces... using callWithRequest is at least safer, but the SavedObjectClient is preferable (unless we can't do so until the Client/Repository split actually happens).

If I did this right, callWithInternalUser should only be used within checkForDuplicateContext, which tries to prevent two Spaces from existing with the same urlContext. To do that correctly, I need to bypass the security and space filters that exist on the SavedObjectsClient.

So the short answer is "we can't do so until the Client/Repository split actually happens"

Gotcha, that makes sense. As long as we have it noted that we'll need to switch this in the future, LGTM

kobelb

LGTM

legrego added WIP Work in progress Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! labels Apr 26, 2018

legrego self-assigned this Apr 26, 2018

legrego force-pushed the space-avatar branch from e906128 to 1f1be38 Compare April 27, 2018 13:01

legrego requested a review from kobelb April 27, 2018 13:05

legrego force-pushed the space-avatar branch from eb5a012 to b0443c6 Compare April 27, 2018 15:38

legrego force-pushed the spaces-phase-1 branch from 2a838cc to e4abf8a Compare May 2, 2018 19:23

legrego mentioned this pull request May 3, 2018

Spaces mapping #18778

Merged

3 tasks

legrego force-pushed the space-avatar branch from b0443c6 to 60b0fb3 Compare May 3, 2018 14:25

legrego changed the title ~~[Spaces WIP] Space Avatar with selector in main Kibana menu~~ [Spaces] Space Avatar with selector in main Kibana menu May 3, 2018

legrego added 3 commits May 3, 2018 15:23

Adds Space Avatar with selector in main Kibana menu

421215c

Move can_redirect_request back into security plugin. Not needed by sp…

6da79c7

…aces

additional tests

dad0d8d

legrego force-pushed the space-avatar branch from 60b0fb3 to dad0d8d Compare May 4, 2018 12:39

legrego mentioned this pull request May 7, 2018

[Spaces] Space-Aware Saved Objects #18862

Merged

legrego removed the WIP Work in progress label May 7, 2018

kobelb reviewed May 7, 2018

View reviewed changes

legrego added 3 commits May 8, 2018 10:08

Determine active space before page load, instead of after page load

8006885

Show notification when navigating to an invalid space

2506398

Fix navigation when basePath is not set

d6ec0f6

legrego mentioned this pull request May 9, 2018

RBAC - Phase 2 (Spaces) #18948

Closed

33 tasks

improved support for displaying a large number of spaces

1b99186

legrego force-pushed the space-avatar branch from c44987e to 1b99186 Compare May 18, 2018 17:34

additional testing

ad5c5dd

kobelb reviewed May 21, 2018

View reviewed changes

kobelb approved these changes May 21, 2018

View reviewed changes

add note to replace call with SOC repository

1ce4cc7

legrego merged commit 50c73b4 into elastic:spaces-phase-1 May 21, 2018

legrego deleted the space-avatar branch April 29, 2019 14:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spaces] Space Avatar with selector in main Kibana menu #18609

[Spaces] Space Avatar with selector in main Kibana menu #18609

legrego commented Apr 26, 2018 •

edited

Loading

kobelb Apr 27, 2018

kobelb May 7, 2018

legrego May 7, 2018

kobelb May 7, 2018

legrego May 7, 2018

kobelb May 7, 2018

legrego May 7, 2018

kobelb May 8, 2018

kobelb May 7, 2018

legrego May 7, 2018

legrego May 8, 2018

kobelb May 8, 2018

kobelb commented May 7, 2018

legrego commented May 8, 2018

rhoboat commented May 10, 2018 •

edited

Loading

legrego commented May 10, 2018

rhoboat commented May 11, 2018

kobelb commented May 11, 2018

legrego commented May 15, 2018

legrego commented May 18, 2018

legrego commented May 18, 2018

kobelb left a comment

kobelb May 21, 2018

legrego May 21, 2018

kobelb May 21, 2018

kobelb left a comment

[Spaces] Space Avatar with selector in main Kibana menu #18609

[Spaces] Space Avatar with selector in main Kibana menu #18609

Conversation

legrego commented Apr 26, 2018 • edited Loading

Space Selection Screen

Main Navigation

Space Selection Modal

Space Management

Quick Visual Walkthrough

TODO:

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

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

kobelb commented May 7, 2018

legrego commented May 8, 2018

rhoboat commented May 10, 2018 • edited Loading

legrego commented May 10, 2018

rhoboat commented May 11, 2018

kobelb commented May 11, 2018

legrego commented May 15, 2018

legrego commented May 18, 2018

legrego commented May 18, 2018

kobelb left a 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

kobelb left a comment

Choose a reason for hiding this comment

legrego commented Apr 26, 2018 •

edited

Loading

rhoboat commented May 10, 2018 •

edited

Loading