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

[SR] Repository list and details UI #33367

Merged
2 changes: 2 additions & 0 deletions x-pack/plugins/infra/types/eui.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ declare module '@elastic/eui' {
loading?: any;
hasActions?: any;
message?: any;
rowProps?: any;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

VSCode was complaining about missing props on EuiInMemoryTable, it used the definition from here

cellProps?: any;
};
export const EuiInMemoryTable: React.SFC<EuiInMemoryTableProps>;
}
2 changes: 2 additions & 0 deletions x-pack/plugins/snapshot_restore/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ export const PLUGIN = {
});
},
};

export const API_BASE_PATH = '/api/snapshot_restore/';
137 changes: 137 additions & 0 deletions x-pack/plugins/snapshot_restore/common/repository_types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not very keen on mixing in the same file constant values and types. Also, the way those are used is still a bit of our old js old patterns, I'll explain 😊

We don't need to declare constants to save guard us from tipos or value update. So we don't need to set a value for FSRepositoryType so we can use it everywhere as a key. When you use them in your switch statement, we have the auto-complete that guides us. And we can only use those values or Typescript will complain. This also means that we don't need all the types imports on top of the file. Take for example documentation_links.ts

Screen Shot 2019-03-20 at 15 09 38

Screen Shot 2019-03-20 at 15 11 10

Another thing is that I would make use of Enum for the different doc URLs. This (and all) enums should go in the constants.ts file, not in the types as they represent values.

export enum RepositoryDocs {
  fs = 'modules-snapshots.html#_shared_file_system_repository',
  readOnly = 'modules-snapshots.html#_read_only_url_repository',
  ...
}

Then we'd only have 1 import, and the above example (in she screenshot) would end up something like this

Screen Shot 2019-03-20 at 15 19 58

The change from const to types would just be

export const FSRepositoryType = 'fs';
export const ReadonlyRepositoryType = 'url';
export const SourceRepositoryType = 'source';
...

// after
export type FSRepository = 'fs'; // no need to add a "Type" at the end, as it can only be used as a type
export type ReadonlyRepository = 'url';
export type SourceRepository = 'source';
...

export type RepositoryType = FSRepository |  ReadonlyRepository | SourceRepository

One last thing 😊 (this is more a style guide to agree on), should we create a folder for types and then exposed them all in a barel file?

So this file would be in common/types/repository.ts. And then we'd have an index.ts file exporting all the types. Then when importing types, it would always be from the same path

import { SomeType} from ../common/types

* 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.
*/

export const FSRepositoryType = 'fs';
Copy link
Contributor

Choose a reason for hiding this comment

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

Like we talked yesterday, this list of constants would be better written with an Enum

export enum RepositoryTypes {
  URL = 'url',
  SOURCE = 'source',
  S3 = 's3',
  ...
}

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 you are viewing an outdated file 😄 I moved this to common/types/ and updated with enums: https://github.com/elastic/kibana/pull/33367/files#diff-5c7d6779b8a84817b082813365634361R1

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah sorry! I wrote the comment before you merged it. Then left the review for a while and we got a race condition! 😄

export const ReadonlyRepositoryType = 'url';
export const SourceRepositoryType = 'source';
export const S3RepositoryType = 's3';
export const HDFSRepositoryType = 'hdfs';
export const AzureRepositoryType = 'azure';
export const GCSRepositoryType = 'gcs';

export const RepositoryTypeDocPath = 'modules-snapshots.html';
export const FSRepositoryTypeDocPath = 'modules-snapshots.html#_shared_file_system_repository';
export const ReadonlyRepositoryTypeDocPath = 'modules-snapshots.html#_read_only_url_repository';
export const SourceRepositoryTypeDocPath = 'modules-snapshots.html#_source_only_repository';
export const S3RepositoryTypeDocPath = 'repository-s3.html';
export const HDFSRepositoryTypeDocPath = 'repository-hdfs.html';
export const AzureRepositoryTypeDocPath = 'repository-azure.html';
export const GCSRepositoryTypeDocPath = 'repository-gcs.html';

export type RepositoryType =
| typeof FSRepositoryType
| typeof ReadonlyRepositoryType
| typeof SourceRepositoryType
| typeof S3RepositoryType
| typeof HDFSRepositoryType
| typeof AzureRepositoryType
| typeof GCSRepositoryType;

export interface FSRepository {
name: string;
type: typeof FSRepositoryType;
settings: {
location: string;
compress?: boolean;
chunk_size?: string | null;
max_restore_bytes_per_sec?: string;
max_snapshot_bytes_per_sec?: string;
readonly?: boolean;
};
}

export interface ReadonlyRepository {
name: string;
type: typeof ReadonlyRepositoryType;
settings: {
url: string;
};
}

export interface S3Repository {
name: string;
type: typeof S3RepositoryType;
settings: {
bucket: string;
client?: string;
base_path?: string;
compress?: boolean;
chunk_size?: string | null;
server_side_encryption?: boolean;
buffer_size?: string;
canned_acl?: string;
storage_class?: string;
};
}

export interface HDFSRepository {
name: string;
type: typeof HDFSRepositoryType;
settings: {
uri: string;
path: string;
load_defaults?: boolean;
compress?: boolean;
chunk_size?: string | null;
['security.principal']?: string;
[key: string]: any; // For conf.* settings
};
}

export interface AzureRepository {
name: string;
type: typeof AzureRepositoryType;
settings: {
client?: string;
container?: string;
base_path?: string;
compress?: boolean;
chunk_size?: string | null;
readonly?: boolean;
location_mode?: string;
};
}

export interface GCSRepository {
name: string;
type: typeof GCSRepositoryType;
settings: {
bucket: string;
client?: string;
base_path?: string;
compress?: boolean;
chunk_size?: string | null;
};
}

export interface SourceRepository<T> {
name: string;
type: typeof SourceRepositoryType;
settings: SourceRepositorySettings<T>;
}

export type SourceRepositorySettings<T> = T extends typeof FSRepositoryType
? FSRepository['settings']
: T extends typeof S3RepositoryType
? S3Repository['settings']
: T extends typeof HDFSRepositoryType
? HDFSRepository['settings']
: T extends typeof AzureRepositoryType
? AzureRepository['settings']
: T extends typeof GCSRepositoryType
? GCSRepository['settings']
: any & {
delegate_type: T;
};

export type Repository<T = null> =
| FSRepository
| ReadonlyRepository
| S3Repository
| HDFSRepository
| AzureRepository
| GCSRepository
| SourceRepository<T>;
3 changes: 2 additions & 1 deletion x-pack/plugins/snapshot_restore/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { API_BASE_PATH } from './common/constants';
import { registerRoutes } from './server/routes/api/register_routes';
import { Core, Plugins } from './shim';

export class Plugin {
public start(core: Core, plugins: Plugins): void {
const router = core.http.createRouter('/api/snapshot_restore/');
const router = core.http.createRouter(API_BASE_PATH);

// Register routes
registerRoutes(router);
Expand Down
28 changes: 11 additions & 17 deletions x-pack/plugins/snapshot_restore/public/app/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,19 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { Component } from 'react';
import React from 'react';
import { Redirect, Route, Switch } from 'react-router-dom';

import { BASE_PATH } from './constants';
import { AppContext } from './services/app_context';

import { SnapshotRestoreHome } from './sections';

export class App extends Component {
public static contextType = AppContext;

public render() {
return (
<div>
<Switch>
<Redirect exact from={`${BASE_PATH}`} to={`${BASE_PATH}/repositories`} />
<Route exact path={`${BASE_PATH}/:section`} component={SnapshotRestoreHome} />
</Switch>
</div>
);
}
}
export const App = () => {
return (
<div>
<Switch>
<Redirect exact from={`${BASE_PATH}`} to={`${BASE_PATH}/repositories`} />
<Route exact path={`${BASE_PATH}/:section/:name?`} component={SnapshotRestoreHome} />
</Switch>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import { AppCore, AppPlugins } from '../../shim';

export interface AppContextInterface {
core: AppCore;
plugins: AppPlugins;
}

export const AppContext = React.createContext<AppContextInterface | null>(null);
export { RepositoryDeleteProvider } from './repository_delete_provider';
export { RepositoryTypeName } from './repository_type_name';
export { SectionError } from './section_error';
export { SectionLoading } from './section_loading';
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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 React, { Fragment, useState } from 'react';
import { Repository } from '../../../common/repository_types';

interface Props {
children: any;
}

export const RepositoryDeleteProvider = ({ children }: Props) => {
const [repositoryNames, setRepositoryNames] = useState<Array<Repository['name']>>([]);
const [isModalOpen, setIsModalOpen] = useState<boolean>(false);
const deleteRepository = (names: Array<Repository['name']>): void => {
setIsModalOpen(true);
setRepositoryNames(names);
};

if (isModalOpen && repositoryNames.length) {
/* placeholder */
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This is how I would have typed the children props:

interface Props {
  children: (fun: deleteRepositoryFunction) => ReactNode;
}

type deleteRepositoryFunction = (names: Array<Repository['name']>) => void;

export const RepositoryDeleteProvider = ({ children }: Props) => {
  const [repositoryNames, setRepositoryNames] = useState<Array<Repository['name']>>([]);
  const [isModalOpen, setIsModalOpen] = useState<boolean>(false);
  const deleteRepository: deleteRepositoryFunction = names => {
    setIsModalOpen(true);
    setRepositoryNames(names);
  };

  ...

return <Fragment>{children(deleteRepository)}</Fragment>;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* 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 React, { Fragment } from 'react';
import {
AzureRepositoryType,
FSRepositoryType,
GCSRepositoryType,
HDFSRepositoryType,
ReadonlyRepositoryType,
RepositoryType,
S3RepositoryType,
SourceRepositoryType,
} from '../../../common/repository_types';
import { AppStateInterface, useAppState } from '../services/app_context';

interface Props {
type: RepositoryType;
delegateType?: RepositoryType;
}

export const RepositoryTypeName = ({ type, delegateType }: Props) => {
const [
{
core: {
i18n: { FormattedMessage },
},
},
] = useAppState() as [AppStateInterface];

const getTypeName = (repositoryType: RepositoryType): JSX.Element => {
switch (repositoryType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Another minor nit, you could use a map instead of a switch to reduce visual noise a bit:

    const map = {
      [FSRepositoryType]: (
        <FormattedMessage
          id="xpack.snapshotRestore.repositoryType.fileSystemTypeName"
          defaultMessage="Shared File System"
        />
      ),

      [ReadonlyRepositoryType]: (
        <FormattedMessage
          id="xpack.snapshotRestore.repositoryType.readonlyTypeName"
          defaultMessage="Read-only URL"
        />
      ),

      [S3RepositoryType]: (
        <FormattedMessage
          id="xpack.snapshotRestore.repositoryType.s3TypeName"
          defaultMessage="AWS S3"
        />
      ),

      /* ... */      
    };

case FSRepositoryType:
return (
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.fileSystemTypeName"
defaultMessage="Shared File System"
/>
);
break;
case ReadonlyRepositoryType:
return (
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.readonlyTypeName"
defaultMessage="Read-only URL"
/>
);
break;
case S3RepositoryType:
return (
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.s3TypeName"
defaultMessage="AWS S3"
/>
);
break;
case HDFSRepositoryType:
return (
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.hdfsTypeName"
defaultMessage="HDFS File System"
/>
);
break;
case AzureRepositoryType:
return (
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.azureTypeName"
defaultMessage="Azure"
/>
);
break;
case GCSRepositoryType:
return (
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.gcsTypeName"
defaultMessage="Google Cloud Storage"
/>
);
break;
case SourceRepositoryType:
return (
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.sourceTypeName"
defaultMessage="Source"
/>
);
break;
default:
return <Fragment>{type}</Fragment>;
break;
}
};

if (type === SourceRepositoryType && delegateType) {
return (
<Fragment>
<FormattedMessage
id="xpack.snapshotRestore.repositoryType.sourceTypeWithDelegateName"
defaultMessage="Source ({delegateType})"
values={{
delegateType: getTypeName(delegateType),
}}
/>
</Fragment>
);
}

return getTypeName(type);
};
Loading