-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Spaces - Client NP Migration, Phase 1 #40856
Changes from 12 commits
f80f888
452d592
b41073c
1677dc4
a014d72
b0bf4dd
c46f283
aed3888
2aa3b71
b039a99
8c161f2
3a010f1
8efbeab
a3e1e59
86a4379
8f398c6
f866d7d
a551a50
e9bdc21
8df1838
9dc0957
6517a60
e314536
75c0b02
67eea7b
3acca45
40b6c9e
444dbb6
13cc9ae
1b50503
6f123b6
6192de1
e41dc60
9482b47
86bc36d
e80e91b
0d1d283
c849fde
e4c0e43
03e93da
7d15190
c3cb211
6cf91d4
8146ddb
b593c70
ebd3147
d2b2452
54f8ce7
f6249ff
b27c7b7
eb2714b
839b9a7
1871ef7
d771522
0186d5f
5b2c16a
03f1687
e29437b
71865a9
5cbb6d9
2ff6cb8
7cfdcb1
bbfc0bd
ffb8a7b
884e545
cb44020
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,9 +11,6 @@ import { createOptionalPlugin } from '../../server/lib/optional_plugin'; | |
// @ts-ignore | ||
import { AuditLogger } from '../../server/lib/audit_logger'; | ||
import mappings from './mappings.json'; | ||
import { wrapError } from './server/lib/errors'; | ||
import { getActiveSpace } from './server/lib/get_active_space'; | ||
import { getSpaceSelectorUrl } from './server/lib/get_space_selector_url'; | ||
import { migrateToKibana660 } from './server/lib/migrations'; | ||
import { plugin } from './server/new_platform'; | ||
import { | ||
|
@@ -51,7 +48,6 @@ export const spaces = (kibana: Record<string, any>) => | |
}, | ||
|
||
uiExports: { | ||
chromeNavControls: ['plugins/spaces/views/nav_control'], | ||
styleSheetPaths: resolve(__dirname, 'public/index.scss'), | ||
managementSections: ['plugins/spaces/views/management'], | ||
apps: [ | ||
|
@@ -75,38 +71,7 @@ export const spaces = (kibana: Record<string, any>) => | |
isNamespaceAgnostic: true, | ||
}, | ||
}, | ||
home: ['plugins/spaces/register_feature'], | ||
injectDefaultVars(server: any) { | ||
return { | ||
spaces: [], | ||
activeSpace: null, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No more There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: it seems Infra still relies on this variable: https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/infra/public/utils/use_kibana_space_id.ts#L14 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! I've restored this variable solely for their use in 0186d5f |
||
spaceSelectorURL: getSpaceSelectorUrl(server.config()), | ||
}; | ||
}, | ||
async replaceInjectedVars( | ||
vars: Record<string, any>, | ||
request: Record<string, any>, | ||
server: Record<string, any> | ||
) { | ||
const spacesClient = await server.plugins.spaces.getScopedSpacesClient(request); | ||
try { | ||
vars.activeSpace = { | ||
valid: true, | ||
space: await getActiveSpace( | ||
spacesClient, | ||
request.getBasePath(), | ||
server.config().get('server.basePath') | ||
), | ||
}; | ||
} catch (e) { | ||
vars.activeSpace = { | ||
valid: false, | ||
error: wrapError(e).output.payload, | ||
}; | ||
} | ||
|
||
return vars; | ||
}, | ||
home: ['plugins/spaces/legacy'], | ||
}, | ||
|
||
async init(server: Server) { | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
import { PluginInitializerContext } from 'src/core/public'; | ||
import { SpacesPlugin } from './plugin'; | ||
|
||
export const plugin = (initializerContext: PluginInitializerContext) => { | ||
return new SpacesPlugin(initializerContext); | ||
}; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,23 @@ | ||||||
/* | ||||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||||||
* or more contributor license agreements. Licensed under the Elastic License; | ||||||
* you may not use this file except in compliance with the Elastic License. | ||||||
*/ | ||||||
|
||||||
import { npSetup, npStart } from 'ui/new_platform'; | ||||||
import { FeatureCatalogueRegistryProvider } from 'ui/registry/feature_catalogue'; | ||||||
import { plugin } from './index'; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. optional nit:
Suggested change
|
||||||
import { SpacesPlugin, PluginsSetup } from './plugin'; | ||||||
|
||||||
const spacesPlugin: SpacesPlugin = plugin({}); | ||||||
|
||||||
const plugins: PluginsSetup = { | ||||||
kibana: { | ||||||
registerCatalogueFeature: fn => { | ||||||
FeatureCatalogueRegistryProvider.register(fn); | ||||||
}, | ||||||
}, | ||||||
}; | ||||||
|
||||||
export const setup = spacesPlugin.setup(npSetup.core); | ||||||
export const start = spacesPlugin.start(npStart.core, plugins); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,58 +4,64 @@ | |||||
* you may not use this file except in compliance with the Elastic License. | ||||||
*/ | ||||||
import { i18n } from '@kbn/i18n'; | ||||||
import { toastNotifications } from 'ui/notify'; | ||||||
import { EventEmitter } from 'events'; | ||||||
import { kfetch } from 'ui/kfetch'; | ||||||
import { NotificationsSetup, HttpSetup } from 'src/core/public'; | ||||||
import { Space } from '../../common/model/space'; | ||||||
|
||||||
export class SpacesManager extends EventEmitter { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: probably we can discuss this in the scope of another PR, but since there are no other consumers at this point, I'm wondering if having
What about exposing something like Also depending on our plans, we can expose I admit that this type of complexity may not be needed in this particular case though, but since it's in a public plugin contract we may want to have a reference NP implementation for things like this at some point :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You raise some very good points here, thanks for taking the time to explain your thoughts! I'll think on this some more and see if I can get rid of the |
||||||
private spaceSelectorURL: string; | ||||||
private activeSpace: Space | undefined; | ||||||
|
||||||
constructor(spaceSelectorURL: string) { | ||||||
constructor( | ||||||
private readonly spaceSelectorURL: string, | ||||||
private readonly http: HttpSetup, | ||||||
private readonly notifications: NotificationsSetup | ||||||
) { | ||||||
super(); | ||||||
this.spaceSelectorURL = spaceSelectorURL; | ||||||
} | ||||||
|
||||||
public async getSpaces(): Promise<Space[]> { | ||||||
return await kfetch({ pathname: '/api/spaces/space' }); | ||||||
return await this.http.get('/api/spaces/space'); | ||||||
} | ||||||
|
||||||
public async getSpace(id: string): Promise<Space> { | ||||||
return await kfetch({ pathname: `/api/spaces/space/${encodeURIComponent(id)}` }); | ||||||
return await this.http.get(`/api/spaces/space/${encodeURIComponent(id)}`); | ||||||
} | ||||||
|
||||||
public async getActiveSpace(forceRefresh: boolean = false): Promise<Space> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: since we don't use
Suggested change
|
||||||
if (!this.activeSpace || forceRefresh) { | ||||||
this.activeSpace = (await this.http.get('/api/spaces/v1/activeSpace')) as Space; | ||||||
} | ||||||
return this.activeSpace; | ||||||
} | ||||||
|
||||||
public async createSpace(space: Space) { | ||||||
legrego marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return await kfetch({ | ||||||
pathname: `/api/spaces/space`, | ||||||
method: 'POST', | ||||||
body: JSON.stringify(space), | ||||||
}); | ||||||
return this.http | ||||||
.post(`/api/spaces/space`, { | ||||||
body: JSON.stringify(space), | ||||||
}) | ||||||
.then(() => this.requestRefresh()); | ||||||
} | ||||||
|
||||||
public async updateSpace(space: Space) { | ||||||
return await kfetch({ | ||||||
pathname: `/api/spaces/space/${encodeURIComponent(space.id)}`, | ||||||
query: { | ||||||
overwrite: true, | ||||||
}, | ||||||
method: 'PUT', | ||||||
body: JSON.stringify(space), | ||||||
}); | ||||||
return this.http | ||||||
.put(`/api/spaces/space/${encodeURIComponent(space.id)}`, { | ||||||
query: { | ||||||
overwrite: true, | ||||||
}, | ||||||
body: JSON.stringify(space), | ||||||
}) | ||||||
.then(() => this.requestRefresh()); | ||||||
} | ||||||
|
||||||
public async deleteSpace(space: Space) { | ||||||
return await kfetch({ | ||||||
pathname: `/api/spaces/space/${encodeURIComponent(space.id)}`, | ||||||
method: 'DELETE', | ||||||
}); | ||||||
return this.http | ||||||
.delete(`/api/spaces/space/${encodeURIComponent(space.id)}`) | ||||||
.then(() => this.requestRefresh()); | ||||||
} | ||||||
|
||||||
public async changeSelectedSpace(space: Space) { | ||||||
await kfetch({ | ||||||
pathname: `/api/spaces/v1/space/${encodeURIComponent(space.id)}/select`, | ||||||
method: 'POST', | ||||||
}) | ||||||
await this.http | ||||||
.post(`/api/spaces/v1/space/${encodeURIComponent(space.id)}/select`) | ||||||
.then(response => { | ||||||
if (response.location) { | ||||||
window.location = response.location; | ||||||
|
@@ -75,7 +81,7 @@ export class SpacesManager extends EventEmitter { | |||||
} | ||||||
|
||||||
public _displayError() { | ||||||
toastNotifications.addDanger({ | ||||||
this.notifications.toasts.addDanger({ | ||||||
title: i18n.translate('xpack.spaces.spacesManager.unableToChangeSpaceWarningTitle', { | ||||||
defaultMessage: 'Unable to change your Space', | ||||||
}), | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'src/core/public'; | ||
import { FeatureCatalogueRegistryFunction } from 'ui/registry/feature_catalogue'; | ||
import { SpacesManager } from './lib'; | ||
import { initSpacesNavControl } from './views/nav_control'; | ||
import { createSpacesFeatureCatalogueEntry } from './register_feature'; | ||
|
||
export interface SpacesPluginStart { | ||
spacesManager: SpacesManager; | ||
} | ||
|
||
export interface PluginsSetup { | ||
kibana: { | ||
registerCatalogueFeature: (fn: FeatureCatalogueRegistryFunction) => void; | ||
}; | ||
} | ||
|
||
export class SpacesPlugin implements Plugin<{}, SpacesPluginStart, PluginsSetup> { | ||
legrego marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private spacesManager: SpacesManager | undefined; | ||
legrego marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// @ts-ignore | ||
constructor(private readonly initializerContext: PluginInitializerContext) {} | ||
|
||
public async start(core: CoreStart, plugins: PluginsSetup) { | ||
const { spaceSelectorUrl } = await core.http.get('/api/spaces/v1/npStart'); | ||
this.spacesManager = new SpacesManager(spaceSelectorUrl, core.http, core.notifications); | ||
|
||
initSpacesNavControl(this.spacesManager, core); | ||
plugins.kibana.registerCatalogueFeature(createSpacesFeatureCatalogueEntry); | ||
|
||
return { | ||
spacesManager: this.spacesManager, | ||
}; | ||
} | ||
|
||
public async setup(core: CoreSetup) {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reducing security's dependency on the spaces plugin, at least for now.