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

Update Project Selection in Serverless Top Navigation #163076

Merged
merged 13 commits into from
Aug 7, 2023
Merged

Update Project Selection in Serverless Top Navigation #163076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3726abc
wip
Dosant Aug 3, 2023
1c962b0
fix lint
Dosant Aug 3, 2023
5c349b9
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 3, 2023
7c28472
fix ts
Dosant Aug 3, 2023
b8adfc0
Merge branch 'd/2023-08-03-change-deployments-link' of github.com:Dos…
Dosant Aug 3, 2023
8b85a2e
fix i18n
Dosant Aug 3, 2023
f8c59e9
fix core test
Dosant Aug 4, 2023
dd35820
Merge branch 'main' of github.com:elastic/kibana into d/2023-08-03-ch…
Dosant Aug 4, 2023
72be0c0
Merge branch 'main' into d/2023-08-03-change-deployments-link
kibanamachine Aug 4, 2023
bd6e7e1
Merge branch 'main' of github.com:elastic/kibana into d/2023-08-03-ch…
Dosant Aug 7, 2023
eebd9fb
allow “projects_url” only in serverless, set default in the config
Dosant Aug 7, 2023
662e886
consistency
Dosant Aug 7, 2023
f84d6b6
fix
Dosant Aug 7, 2023
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
7 changes: 7 additions & 0 deletions packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,11 @@ export class ChromeService {
projectNavigation.setProjectHome(homeHref);
};

const setProjectsUrl = (projectsUrl: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if at some point replacing all the setters with a single one would make sense... Maybe not as that function would be a big one instead of many small functions... 🤔

updateProjectConfig({ ... partial configs here });

validateChromeStyle();
projectNavigation.setProjectsUrl(projectsUrl);
};

const isIE = () => {
const ua = window.navigator.userAgent;
const msie = ua.indexOf('MSIE '); // IE 10 or older
Expand Down Expand Up @@ -317,6 +322,7 @@ export class ChromeService {
loadingCount$={http.getLoadingCount$()}
headerBanner$={headerBanner$.pipe(takeUntil(this.stop$))}
homeHref$={projectNavigation.getProjectHome$()}
projectsUrl$={projectNavigation.getProjectsUrl$()}
docLinks={docLinks}
kibanaVersion={injectedMetadata.getKibanaVersion()}
prependBasePath={http.basePath.prepend}
Expand Down Expand Up @@ -444,6 +450,7 @@ export class ChromeService {
getChromeStyle$: () => chromeStyle$.pipe(takeUntil(this.stop$)),
project: {
setHome: setProjectHome,
setProjectsUrl,
setNavigation: setProjectNavigation,
setSideNavComponent: setProjectSideNavComponent,
setBreadcrumbs: setProjectBreadcrumbs,
Expand Down
7 changes: 7 additions & 0 deletions .../chrome/core-chrome-browser-internal/src/project_navigation/project_navigation_service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class ProjectNavigationService {
current: SideNavComponent | null;
}>({ current: null });
private projectHome$ = new BehaviorSubject<string | undefined>(undefined);
private projectsUrl$ = new BehaviorSubject<string | undefined>(undefined);
private projectNavigation$ = new BehaviorSubject<ChromeProjectNavigation | undefined>(undefined);
private activeNodes$ = new BehaviorSubject<ChromeProjectNavigationNode[][]>([]);
private projectNavigationNavTreeFlattened: Record<string, ChromeProjectNavigationNode> = {};
Expand Down Expand Up @@ -66,6 +67,12 @@ export class ProjectNavigationService {
getProjectHome$: () => {
return this.projectHome$.asObservable();
},
setProjectsUrl: (projectsUrl: string) => {
this.projectsUrl$.next(projectsUrl);
},
getProjectsUrl$: () => {
return this.projectsUrl$.asObservable();
},
setProjectNavigation: (projectNavigation: ChromeProjectNavigation) => {
this.projectNavigation$.next(projectNavigation);
this.projectNavigationNavTreeFlattened = flattenNav(projectNavigation.navigationTree);
Expand Down
6 changes: 6 additions & 0 deletions packages/core/chrome/core-chrome-browser-internal/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ export interface InternalChromeStart extends ChromeStart {
*/
setHome(homeHref: string): void;

/**
* Sets the cloud's projects page.
* @param projectsUrl
*/
setProjectsUrl(projectsUrl: string): void;

/**
* Sets the project navigation config to be used for rendering project navigation.
* It is used for default project sidenav, project breadcrumbs, tracking active deep link.
Expand Down
12 changes: 12 additions & 0 deletions packages/core/chrome/core-chrome-browser-internal/src/ui/project/header.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ describe('Header', () => {
helpSupportUrl$: Rx.of('app/help'),
helpMenuLinks$: Rx.of([]),
homeHref$: Rx.of('app/home'),
projectsUrl$: Rx.of('/projects/'),
kibanaVersion: '8.9',
loadingCount$: Rx.of(0),
navControlsLeft$: Rx.of([]),
Expand Down Expand Up @@ -71,4 +72,15 @@ describe('Header', () => {
await toggleNav();
await toggleNav();
});

it('displays the link to projects', async () => {
render(
<ProjectHeader {...mockProps}>
<EuiHeader>Hello, world!</EuiHeader>
</ProjectHeader>
);

const projectsLink = await screen.getByTestId('projectsLink');
expect(projectsLink).toHaveAttribute('href', '/projects/');
});
});
10 changes: 6 additions & 4 deletions packages/core/chrome/core-chrome-browser-internal/src/ui/project/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ const headerStrings = {
}),
},
cloud: {
linkToDeployments: i18n.translate('core.ui.primaryNav.cloud.linkToDeployments', {
defaultMessage: 'My deployments',
linkToProjects: i18n.translate('core.ui.primaryNav.cloud.linkToProjects', {
defaultMessage: 'Projects',
}),
},
nav: {
Expand All @@ -100,6 +100,7 @@ export interface Props {
helpSupportUrl$: Observable<string>;
helpMenuLinks$: Observable<ChromeHelpMenuLink[]>;
homeHref$: Observable<string | undefined>;
projectsUrl$: Observable<string | undefined>;
kibanaVersion: string;
application: InternalApplicationStart;
loadingCount$: ReturnType<HttpStart['getLoadingCount$']>;
Expand Down Expand Up @@ -175,6 +176,7 @@ export const ProjectHeader = ({
const [isOpen, setIsOpen] = useLocalStorage(LOCAL_STORAGE_IS_OPEN_KEY, true);
const toggleCollapsibleNavRef = createRef<HTMLButtonElement & { euiAnimate: () => void }>();
const headerActionMenuMounter = useHeaderActionMenuMounter(observables.actionMenu$);
const projectsUrl = useObservable(observables.projectsUrl$);

return (
<>
Expand Down Expand Up @@ -233,8 +235,8 @@ export const ProjectHeader = ({
</EuiHeaderSectionItem>

<EuiHeaderSectionItem>
<EuiHeaderLink href="https://cloud.elastic.co/deployments">
{headerStrings.cloud.linkToDeployments}
<EuiHeaderLink href={projectsUrl} data-test-subj={'projectsLink'}>
{headerStrings.cloud.linkToProjects}
</EuiHeaderLink>
</EuiHeaderSectionItem>

Expand Down
1 change: 1 addition & 0 deletions packages/core/chrome/core-chrome-browser-mocks/src/chrome_service.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const createStartContractMock = () => {
setChromeStyle: jest.fn(),
project: {
setHome: jest.fn(),
setProjectsUrl: jest.fn(),
setNavigation: jest.fn(),
setSideNavComponent: jest.fn(),
setBreadcrumbs: jest.fn(),
Expand Down
1 change: 1 addition & 0 deletions test/plugin_functional/test_suites/core_plugins/rendering.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
'xpack.cloud.profile_url (string)',
'xpack.cloud.performance_url (string)',
'xpack.cloud.users_and_roles_url (string)',
'xpack.cloud.projects_url (any)', // It's a string (any because schema.conditional)
// can't be used to infer urls or customer id from the outside
'xpack.cloud.serverless.project_id (string)',
'xpack.discoverEnhanced.actions.exploreDataInChart.enabled (boolean)',
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/cloud/public/plugin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const baseConfig = {
deployment_url: '/abc123',
profile_url: '/user/settings/',
organization_url: '/account/',
projects_url: '/projects/',
};

describe('Cloud Plugin', () => {
Expand Down Expand Up @@ -60,6 +61,11 @@ describe('Cloud Plugin', () => {
expect(setup.deploymentUrl).toBe('https://cloud.elastic.co/abc123');
});

it('exposes projectsUrl', () => {
const { setup } = setupPlugin();
expect(setup.projectsUrl).toBe('https://cloud.elastic.co/projects/');
});

it('exposes snapshotsUrl', () => {
const { setup } = setupPlugin();
expect(setup.snapshotsUrl).toBe('https://cloud.elastic.co/abc123/elasticsearch/snapshots/');
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/cloud/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface CloudConfigType {
base_url?: string;
profile_url?: string;
deployment_url?: string;
projects_url?: string;
billing_url?: string;
organization_url?: string;
users_and_roles_url?: string;
Expand All @@ -41,6 +42,7 @@ interface CloudUrls {
snapshotsUrl?: string;
performanceUrl?: string;
usersAndRolesUrl?: string;
projectsUrl?: string;
}

export class CloudPlugin implements Plugin<CloudSetup> {
Expand Down Expand Up @@ -121,6 +123,7 @@ export class CloudPlugin implements Plugin<CloudSetup> {
organizationUrl,
performanceUrl,
usersAndRolesUrl,
projectsUrl,
} = this.getCloudUrls();

let decodedId: DecodedCloudId | undefined;
Expand All @@ -136,6 +139,7 @@ export class CloudPlugin implements Plugin<CloudSetup> {
deploymentUrl,
profileUrl,
organizationUrl,
projectsUrl,
elasticsearchUrl: decodedId?.elasticsearchUrl,
kibanaUrl: decodedId?.kibanaUrl,
isServerlessEnabled: this.isServerlessEnabled,
Expand All @@ -158,6 +162,7 @@ export class CloudPlugin implements Plugin<CloudSetup> {
base_url: baseUrl,
performance_url: performanceUrl,
users_and_roles_url: usersAndRolesUrl,
projects_url: projectsUrl,
} = this.config;

const fullCloudDeploymentUrl = getFullCloudUrl(baseUrl, deploymentUrl);
Expand All @@ -166,6 +171,7 @@ export class CloudPlugin implements Plugin<CloudSetup> {
const fullCloudOrganizationUrl = getFullCloudUrl(baseUrl, organizationUrl);
const fullCloudPerformanceUrl = getFullCloudUrl(baseUrl, performanceUrl);
const fullCloudUsersAndRolesUrl = getFullCloudUrl(baseUrl, usersAndRolesUrl);
const fullCloudProjectsUrl = getFullCloudUrl(baseUrl, projectsUrl);
const fullCloudSnapshotsUrl = `${fullCloudDeploymentUrl}/${CLOUD_SNAPSHOTS_PATH}`;

return {
Expand All @@ -176,6 +182,7 @@ export class CloudPlugin implements Plugin<CloudSetup> {
snapshotsUrl: fullCloudSnapshotsUrl,
performanceUrl: fullCloudPerformanceUrl,
usersAndRolesUrl: fullCloudUsersAndRolesUrl,
projectsUrl: fullCloudProjectsUrl,
};
}
}
8 changes: 8 additions & 0 deletions x-pack/plugins/cloud/public/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export interface CloudStart {
* The full URL to the users and roles page on Elastic Cloud. Undefined if not running on Cloud.
*/
usersAndRolesUrl?: string;
/**
* The full URL to the serverless projects page on Elastic Cloud. Undefined if not running in Serverless.
*/
projectsUrl?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about this, because this could also go inside serverless.*
The problem with putting it inside serverless is that it would be tricky to test this locally as serveless.* must specify projectid

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is ok here. Serverless is Cloud. Now if we want to group the urls under a "serverless" parent that could be an option

interface CloudStart {
  serverless: {
    projectsUrl?: string;
  }
  ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is already serverless parent config, but I am on a fence about putting it there because a valid servleless config requires projectid and the testing local config that just includes serverless.prjectsUrl would be considered invalid

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for hijacking, but will projectsUrl or at least baseUrl be exposed on the server-side contract? We need this on the server side to implement #162887

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think exposing all URLs server-side makes sense; It shouldn't be a problem as they are all static. But I think it better to do in a separate pr

Copy link
Member

Choose a reason for hiding this comment

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

I think exposing all URLs server-side makes sense; It shouldn't be a problem as they are all static. But I think it better to do in a separate pr

Thanks! Do you need a GitHub issue for that, or is it already on your radar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@azasypkin, I am passing by this code myself 😅, I think this is for the @elastic/kibana-core team and I was just sharing my opinion on exposing the URLs. If they agree, I can help out and follow up after this pr

Copy link
Member

Choose a reason for hiding this comment

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

Roger that @Dosant! Then, forwarding my question to @elastic/kibana-core - do you folks want me to file a GitHub issue to expose these configuration flags via the server-side contract, or is it already on your radar (unless there is a reason not to expose this)? We need this for #162887 that is targeting Serverless MVP.

Copy link
Contributor

@jloleysens jloleysens Aug 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

+1 from me to expose URL values in cloud plugin's server side. Might be good to have an issue to know which values you need, thanks!

/**
* The full URL to the elasticsearch cluster.
*/
Expand Down Expand Up @@ -90,6 +94,10 @@ export interface CloudSetup {
* The full URL to the deployment management page on Elastic Cloud. Undefined if not running on Cloud.
*/
deploymentUrl?: string;
/**
* The full URL to the serverless projects page on Elastic Cloud. Undefined if not running in Serverless.
*/
projectsUrl?: string;
/**
* The full URL to the user profile page on Elastic Cloud. Undefined if not running on Cloud.
*/
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/cloud/server/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ const configSchema = schema.object({
users_and_roles_url: schema.maybe(schema.string()),
organization_url: schema.maybe(schema.string()),
profile_url: schema.maybe(schema.string()),
projects_url: schema.conditional(
schema.contextRef('serverless'),
true,
schema.string({ defaultValue: '/projects/' }),
schema.never()
),
trial_end_date: schema.maybe(schema.string()),
is_elastic_staff_owned: schema.maybe(schema.boolean()),
serverless: schema.maybe(
Expand All @@ -51,6 +57,7 @@ export const config: PluginConfigDescriptor<CloudConfigType> = {
performance_url: true,
organization_url: true,
profile_url: true,
projects_url: true,
trial_end_date: true,
is_elastic_staff_owned: true,
serverless: {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/serverless/kibana.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"requiredPlugins": [
"kibanaReact",
"management",
"cloud"
],
"optionalPlugins": [],
"requiredBundles": []
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/serverless/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ export class ServerlessPlugin

// Casting the "chrome.projects" service to an "internal" type: this is intentional to obscure the property from Typescript.
const { project } = core.chrome as InternalChromeStart;
if (dependencies.cloud.projectsUrl) {
project.setProjectsUrl(dependencies.cloud.projectsUrl);
}

return {
setSideNavComponent: (sideNavigationComponent) =>
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/serverless/public/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
ChromeProjectNavigationNode,
} from '@kbn/core-chrome-browser';
import type { ManagementSetup, ManagementStart } from '@kbn/management-plugin/public';
import type { CloudSetup, CloudStart } from '@kbn/cloud-plugin/public';
import type { Observable } from 'rxjs';

// eslint-disable-next-line @typescript-eslint/no-empty-interface
Expand All @@ -31,8 +32,10 @@ export interface ServerlessPluginStart {

export interface ServerlessPluginSetupDependencies {
management: ManagementSetup;
cloud: CloudSetup;
}

export interface ServerlessPluginStartDependencies {
management: ManagementStart;
cloud: CloudStart;
}
1 change: 1 addition & 0 deletions x-pack/plugins/serverless/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@
"@kbn/core-chrome-browser",
"@kbn/core-chrome-browser-internal",
"@kbn/i18n-react",
"@kbn/cloud-plugin",
]
}
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/fr-FR.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,6 @@
"core.ui.overlays.banner.attentionTitle": "Attention",
"core.ui.overlays.banner.closeButtonLabel": "Fermer",
"core.ui.primaryNav.addData": "Ajouter des intégrations",
"core.ui.primaryNav.cloud.linkToDeployments": "Mes déploiements",
"core.ui.primaryNav.goToHome.ariaLabel": "Accéder à la page d’accueil",
"core.ui.primaryNav.pinnedLinksAriaLabel": "Liens épinglés",
"core.ui.primaryNav.screenReaderLabel": "Principale",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,6 @@
"core.ui.overlays.banner.attentionTitle": "注意",
"core.ui.overlays.banner.closeButtonLabel": "閉じる",
"core.ui.primaryNav.addData": "統合の追加",
"core.ui.primaryNav.cloud.linkToDeployments": "マイデプロイ",
"core.ui.primaryNav.goToHome.ariaLabel": "ホームページに移動",
"core.ui.primaryNav.pinnedLinksAriaLabel": "ピン留めされたリンク",
"core.ui.primaryNav.screenReaderLabel": "プライマリ",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,6 @@
"core.ui.overlays.banner.attentionTitle": "注意",
"core.ui.overlays.banner.closeButtonLabel": "关闭",
"core.ui.primaryNav.addData": "添加集成",
"core.ui.primaryNav.cloud.linkToDeployments": "我的部署",
"core.ui.primaryNav.goToHome.ariaLabel": "前往主页",
"core.ui.primaryNav.pinnedLinksAriaLabel": "置顶链接",
"core.ui.primaryNav.screenReaderLabel": "主分片",
Expand Down